From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
The strict flag in struct pinmux_ops disallows the usage of the same pin
as a GPIO and for another function. Without it, a rouge user-space
process with enough privileges (or even a buggy driver) can request a
used pin as GPIO and drive it, potentially confusing devices or even
crashing the system. Set it globally for all pinctrl-msm users.
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Tested-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/pinctrl/qcom/pinctrl-msm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c
index a5f69464827119dfe2a7781b558094b283fca215..1751d838ce95d6138c824b90098f74891dec7656 100644
--- a/drivers/pinctrl/qcom/pinctrl-msm.c
+++ b/drivers/pinctrl/qcom/pinctrl-msm.c
@@ -268,6 +268,7 @@ static const struct pinmux_ops msm_pinmux_ops = {
.function_is_gpio = pinmux_generic_function_is_gpio,
.gpio_request_enable = msm_pinmux_request_gpio,
.set_mux = msm_pinmux_set_mux,
+ .strict = true,
};
static int msm_config_reg(struct msm_pinctrl *pctrl,
--
2.48.1
On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > The strict flag in struct pinmux_ops disallows the usage of the same pin > as a GPIO and for another function. Without it, a rouge user-space > process with enough privileges (or even a buggy driver) can request a > used pin as GPIO and drive it, potentially confusing devices or even > crashing the system. Set it globally for all pinctrl-msm users. How does this keep (or allow) I²C generic recovery mechanism to work? -- With Best Regards, Andy Shevchenko
On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > as a GPIO and for another function. Without it, a rouge user-space > > process with enough privileges (or even a buggy driver) can request a > > used pin as GPIO and drive it, potentially confusing devices or even > > crashing the system. Set it globally for all pinctrl-msm users. > > How does this keep (or allow) I涎 generic recovery mechanism to work? > What even is the "generic recovery mechanism"? That's the first time I'm hearing this name. Bartosz
On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > as a GPIO and for another function. Without it, a rouge user-space > > > process with enough privileges (or even a buggy driver) can request a > > > used pin as GPIO and drive it, potentially confusing devices or even > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > How does this keep (or allow) I涎 generic recovery mechanism to work? > > What even is the "generic recovery mechanism"? That's the first time > I'm hearing this name. "I²C generic recovery mechanism" (I dunno why you put away the I²C keyword). -- With Best Regards, Andy Shevchenko
Hi Andy, On Tue, 2 Sept 2025 at 22:46, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > > > How does this keep (or allow) I涎 generic recovery mechanism to work? > > > > What even is the "generic recovery mechanism"? That's the first time > > I'm hearing this name. > > "I²C generic recovery mechanism" (I dunno why you put away the I²C keyword). Your original email had: Content-Type: text/plain; charset=iso-8859-1 Bartosz' has: Content-Type: text/plain; charset="UTF-8" Note that the Gmail web interface does show me the Kanji character instead of the intended "²C". Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > process with enough privileges (or even a buggy driver) can request a > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > How does this keep (or allow) I涎 generic recovery mechanism to work? > > > > What even is the "generic recovery mechanism"? That's the first time > > I'm hearing this name. > > "I²C generic recovery mechanism" (I dunno why you put away the I²C keyword). > For some reason it was displayed like that[1] in my client. I see it's correct in lore though. Anyway, what is your point? I don't think it has any impact on this. Bartosz [1] https://postimg.cc/7GFtxzzR
On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > <andriy.shevchenko@intel.com> wrote: > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: ... > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > Anyway, what is your point? I don't think it has any impact on this. If we have a group of pins that are marked as I²C, and we want to use recovery via GPIOs, would it be still possible to request as GPIO when controller driver is in the strict mode? -- With Best Regards, Andy Shevchenko
On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > <andy.shevchenko@gmail.com> wrote: > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > <andriy.shevchenko@intel.com> wrote: > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > ... > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > Anyway, what is your point? I don't think it has any impact on this. > > If we have a group of pins that are marked as I²C, and we want to use recovery > via GPIOs, would it be still possible to request as GPIO when controller driver > is in the strict mode? > Yes, if you mark that function as a "GPIO" function in the pin controller driver. Bartosz
On Wed, Sep 03, 2025 at 12:34:00PM +0200, Bartosz Golaszewski wrote: > On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > > <andy.shevchenko@gmail.com> wrote: > > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: ... > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > > > Anyway, what is your point? I don't think it has any impact on this. > > > > If we have a group of pins that are marked as I²C, and we want to use recovery > > via GPIOs, would it be still possible to request as GPIO when controller driver > > is in the strict mode? > > Yes, if you mark that function as a "GPIO" function in the pin > controller driver. How would it prevent from requesting from user space? -- With Best Regards, Andy Shevchenko
On Wed, Sep 3, 2025 at 12:38 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 12:34:00PM +0200, Bartosz Golaszewski wrote: > > On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > > > <andy.shevchenko@gmail.com> wrote: > > > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > ... > > > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > > > > > Anyway, what is your point? I don't think it has any impact on this. > > > > > > If we have a group of pins that are marked as I²C, and we want to use recovery > > > via GPIOs, would it be still possible to request as GPIO when controller driver > > > is in the strict mode? > > > > Yes, if you mark that function as a "GPIO" function in the pin > > controller driver. > > How would it prevent from requesting from user space? > It wouldn't, we don't discriminate between user-space and in-kernel GPIO users. A function either is a GPIO or isn't. Can you point me to the driver you're thinking about or is this a purely speculative question? Bartosz
On Wed, Sep 03, 2025 at 12:41:48PM +0200, Bartosz Golaszewski wrote: > On Wed, Sep 3, 2025 at 12:38 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 12:34:00PM +0200, Bartosz Golaszewski wrote: > > > On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko > > > <andriy.shevchenko@intel.com> wrote: > > > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > > > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > > > > <andy.shevchenko@gmail.com> wrote: > > > > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: ... > > > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > > > > > > > Anyway, what is your point? I don't think it has any impact on this. > > > > > > > > If we have a group of pins that are marked as I²C, and we want to use recovery > > > > via GPIOs, would it be still possible to request as GPIO when controller driver > > > > is in the strict mode? > > > > > > Yes, if you mark that function as a "GPIO" function in the pin > > > controller driver. > > > > How would it prevent from requesting from user space? > > It wouldn't, we don't discriminate between user-space and in-kernel > GPIO users. A function either is a GPIO or isn't. Can you point me to > the driver you're thinking about or is this a purely speculative > question? The recovery mechanism is in I²C core and many drivers use that. I'm not aware of Qualcomm drivers in particular. But mechanism is in use in I²C DesignWare which is distributed a lot among platforms, so using word 'purely' is incorrect, and word 'speculative' is a bit strong, but you can think of the issue coming later on when somebody does something like this. The same applies to the in-band wakeup UART mechanism. Which means that with this series we will relax it back anyway for the above mentioned cases. (Not sure, but SPI DesignWare requires programming SPI native chip selects even if the GPIO is used for that, this might have also some implications, but here it's for real 'purely speculative'.) -- With Best Regards, Andy Shevchenko
On Wed, Sep 03, 2025 at 01:53:00PM +0300, Andy Shevchenko wrote: > (Not sure, but SPI DesignWare requires programming SPI native chip selects even > if the GPIO is used for that, this might have also some implications, but here > it's for real 'purely speculative'.) It is very common for SPI controllers to absolutely require setting a chip select in order to do a transfer, when using GPIO chip selects with such controllers the standard practice is to just not mux out the chip select signal from the controller and do everything in software. The IP will be doing it's thing with it's chip select signal but that signal is never actually connected to anything.
On Wed, Sep 3, 2025 at 12:53 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 12:41:48PM +0200, Bartosz Golaszewski wrote: > > On Wed, Sep 3, 2025 at 12:38 PM Andy Shevchenko > > <andriy.shevchenko@intel.com> wrote: > > > On Wed, Sep 03, 2025 at 12:34:00PM +0200, Bartosz Golaszewski wrote: > > > > On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko > > > > <andriy.shevchenko@intel.com> wrote: > > > > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > > > > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > > > > > <andy.shevchenko@gmail.com> wrote: > > > > > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: > > ... > > > > > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > > > > > > > > > Anyway, what is your point? I don't think it has any impact on this. > > > > > > > > > > If we have a group of pins that are marked as I²C, and we want to use recovery > > > > > via GPIOs, would it be still possible to request as GPIO when controller driver > > > > > is in the strict mode? > > > > > > > > Yes, if you mark that function as a "GPIO" function in the pin > > > > controller driver. > > > > > > How would it prevent from requesting from user space? > > > > It wouldn't, we don't discriminate between user-space and in-kernel > > GPIO users. A function either is a GPIO or isn't. Can you point me to > > the driver you're thinking about or is this a purely speculative > > question? > > The recovery mechanism is in I²C core and many drivers use that. > I'm not aware of Qualcomm drivers in particular. But mechanism is > in use in I²C DesignWare which is distributed a lot among platforms, > so using word 'purely' is incorrect, and word 'speculative' is a bit > strong, but you can think of the issue coming later on when somebody > does something like this. > > The same applies to the in-band wakeup UART mechanism. > > Which means that with this series we will relax it back anyway for > the above mentioned cases. > > (Not sure, but SPI DesignWare requires programming SPI native chip selects even > if the GPIO is used for that, this might have also some implications, but here > it's for real 'purely speculative'.) > The high-level answer is: yes, a pin that will be used by GPIOLIB needs the function it's muxed to, to be marked as "GPIOable" in its parent pin controller if it's strict. That's still better than the current situation. I can imagine we could differentiate between in-kernel and user-space users of GPIOs and then make it impossible for the latter to request certain pins while they could still be requested in the kernel but that's outside of the scope of this series. I don't see why this would stop these patches though, as they don't break anything unless you decide to make your pin controller strict in which situation you'd need to verify which functions can GPIOs anyway. Bartosz
On Wed, Sep 03, 2025 at 01:05:27PM +0200, Bartosz Golaszewski wrote: > On Wed, Sep 3, 2025 at 12:53 PM Andy Shevchenko > <andriy.shevchenko@intel.com> wrote: > > On Wed, Sep 03, 2025 at 12:41:48PM +0200, Bartosz Golaszewski wrote: > > > On Wed, Sep 3, 2025 at 12:38 PM Andy Shevchenko > > > <andriy.shevchenko@intel.com> wrote: > > > > On Wed, Sep 03, 2025 at 12:34:00PM +0200, Bartosz Golaszewski wrote: > > > > > On Wed, Sep 3, 2025 at 12:22 PM Andy Shevchenko > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > On Wed, Sep 03, 2025 at 09:33:34AM +0200, Bartosz Golaszewski wrote: > > > > > > > On Tue, Sep 2, 2025 at 10:46 PM Andy Shevchenko > > > > > > > <andy.shevchenko@gmail.com> wrote: > > > > > > > > On Tue, Sep 2, 2025 at 8:42 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote: > > > > > > > > > On Tue, Sep 2, 2025 at 4:38 PM Andy Shevchenko > > > > > > > > > <andriy.shevchenko@intel.com> wrote: > > > > > > > > > > On Tue, Sep 02, 2025 at 01:59:25PM +0200, Bartosz Golaszewski wrote: ... > > > > > > > > > > > The strict flag in struct pinmux_ops disallows the usage of the same pin > > > > > > > > > > > as a GPIO and for another function. Without it, a rouge user-space > > > > > > > > > > > process with enough privileges (or even a buggy driver) can request a > > > > > > > > > > > used pin as GPIO and drive it, potentially confusing devices or even > > > > > > > > > > > crashing the system. Set it globally for all pinctrl-msm users. > > > > > > > > > > > > > > > > > > > > How does this keep (or allow) I²C generic recovery mechanism to work? > > > > > > > > > > > > > > Anyway, what is your point? I don't think it has any impact on this. > > > > > > > > > > > > If we have a group of pins that are marked as I²C, and we want to use recovery > > > > > > via GPIOs, would it be still possible to request as GPIO when controller driver > > > > > > is in the strict mode? > > > > > > > > > > Yes, if you mark that function as a "GPIO" function in the pin > > > > > controller driver. > > > > > > > > How would it prevent from requesting from user space? > > > > > > It wouldn't, we don't discriminate between user-space and in-kernel > > > GPIO users. A function either is a GPIO or isn't. Can you point me to > > > the driver you're thinking about or is this a purely speculative > > > question? > > > > The recovery mechanism is in I²C core and many drivers use that. > > I'm not aware of Qualcomm drivers in particular. But mechanism is > > in use in I²C DesignWare which is distributed a lot among platforms, > > so using word 'purely' is incorrect, and word 'speculative' is a bit > > strong, but you can think of the issue coming later on when somebody > > does something like this. > > > > The same applies to the in-band wakeup UART mechanism. > > > > Which means that with this series we will relax it back anyway for > > the above mentioned cases. > > > > (Not sure, but SPI DesignWare requires programming SPI native chip selects even > > if the GPIO is used for that, this might have also some implications, but here > > it's for real 'purely speculative'.) > > The high-level answer is: yes, a pin that will be used by GPIOLIB > needs the function it's muxed to, to be marked as "GPIOable" in its > parent pin controller if it's strict. That's still better than the > current situation. > > I can imagine we could differentiate between in-kernel and user-space > users of GPIOs and then make it impossible for the latter to request > certain pins while they could still be requested in the kernel but > that's outside of the scope of this series. > > I don't see why this would stop these patches though, as they don't > break anything unless you decide to make your pin controller strict in > which situation you'd need to verify which functions can GPIOs anyway. It can't anyway, Linus already applied :-) -- With Best Regards, Andy Shevchenko
On Wed, Sep 3, 2025 at 1:51 PM Andy Shevchenko <andriy.shevchenko@intel.com> wrote: > > > > > I don't see why this would stop these patches though, as they don't > > break anything unless you decide to make your pin controller strict in > > which situation you'd need to verify which functions can GPIOs anyway. > > It can't anyway, Linus already applied :-) > This would be the third time he did it so it's not like it's carved in stone. I see your point but I think we should cross that bridge when we get there. Shouldn't be too hard, we already have an entry point: gpiod_request_user() where we could set an additional flag marking the descriptor as requested by user-space in which case, when we get to the relevant pinctrl code, it could check this and refuse based on whether the descriptor was requested from within the kernel or from outside. Bart
© 2016 - 2025 Red Hat, Inc.