| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <dfe03f88-407e-4ef1-ad30-42db53bbd4e4@samsung.com>
Date: Wed, 19 Feb 2025 09:38:34 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: Bartosz Golaszewski <brgl@...ev.pl>, Linus Walleij
<linus.walleij@...aro.org>
Cc: linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, Bartosz
Golaszewski <bartosz.golaszewski@...aro.org>, stable@...r.kernel.org
Subject: Re: [PATCH 1/8] gpiolib: check the return value of
gpio_chip::get_direction()
Hi Bartosz,
On 10.02.2025 11:51, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> As per the API contract - gpio_chip::get_direction() may fail and return
> a negative error number. However, we treat it as if it always returned 0
> or 1. Check the return value of the callback and propagate the error
> number up the stack.
>
> Cc: stable@...r.kernel.org
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> ---
> drivers/gpio/gpiolib.c | 44 +++++++++++++++++++++++++++++---------------
> 1 file changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 679ed764cb14..5d3774dc748b 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -1057,8 +1057,11 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
> desc->gdev = gdev;
>
> if (gc->get_direction && gpiochip_line_is_valid(gc, desc_index)) {
> - assign_bit(FLAG_IS_OUT,
> - &desc->flags, !gc->get_direction(gc, desc_index));
> + ret = gc->get_direction(gc, desc_index);
> + if (ret < 0)
> + goto err_cleanup_desc_srcu;
> +
> + assign_bit(FLAG_IS_OUT, &desc->flags, !ret);
> } else {
> assign_bit(FLAG_IS_OUT,
> &desc->flags, !gc->direction_input);
This change breaks bcm2835 pincontrol/gpio driver (and probably others)
in next-20250218. The problem is that some gpio lines are initially
configured as alternate function (i.e. uart) and .get_direction returns
-EINVAL for them, what in turn causes the whole gpio chip fail to
register. Here is the log with WARN_ON() added to line
drivers/pinctrl/bcm/pinctrl-bcm2835.c:350 from Raspberry Pi 4B:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/pinctrl/bcm/pinctrl-bcm2835.c:350
bcm2835_gpio_get_direction+0x80/0x98
Modules linked in:
CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted
6.14.0-rc3-next-20250218-dirty #9817
Hardware name: Raspberry Pi 4 Model B (DT)
pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : bcm2835_gpio_get_direction+0x80/0x98
lr : bcm2835_gpio_get_direction+0x18/0x98
...
Call trace:
bcm2835_gpio_get_direction+0x80/0x98 (P)
gpiochip_add_data_with_key+0x874/0xef0
bcm2835_pinctrl_probe+0x354/0x53c
platform_probe+0x68/0xdc
really_probe+0xbc/0x298
__driver_probe_device+0x78/0x12c
driver_probe_device+0xdc/0x164
__driver_attach+0x9c/0x1ac
bus_for_each_dev+0x74/0xd4
driver_attach+0x24/0x30
bus_add_driver+0xe4/0x208
driver_register+0x60/0x128
__platform_driver_register+0x24/0x30
bcm2835_pinctrl_driver_init+0x20/0x2c
do_one_initcall+0x64/0x308
kernel_init_freeable+0x280/0x4e8
kernel_init+0x20/0x1d8
ret_from_fork+0x10/0x20
irq event stamp: 100380
hardirqs last enabled at (100379): [<ffff8000812d7d5c>]
_raw_spin_unlock_irqrestore+0x74/0x78
hardirqs last disabled at (100380): [<ffff8000812c8918>] el1_dbg+0x24/0x8c
softirqs last enabled at (93674): [<ffff80008005ed4c>]
handle_softirqs+0x4c4/0x4dc
softirqs last disabled at (93669): [<ffff8000800105a0>]
__do_softirq+0x14/0x20
---[ end trace 0000000000000000 ]---
gpiochip_add_data_with_key: GPIOs 512..569 (pinctrl-bcm2711) failed to
register, -22
pinctrl-bcm2835 fe200000.gpio: could not add GPIO chip
pinctrl-bcm2835 fe200000.gpio: probe with driver pinctrl-bcm2835
failed with error -22
Any suggestions how to fix this issue? Should we add
GPIO_LINE_DIRECTION_UNKNOWN?
> @@ -2728,13 +2731,18 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
> if (guard.gc->direction_input) {
> ret = guard.gc->direction_input(guard.gc,
> gpio_chip_hwgpio(desc));
> - } else if (guard.gc->get_direction &&
> - (guard.gc->get_direction(guard.gc,
> - gpio_chip_hwgpio(desc)) != 1)) {
> - gpiod_warn(desc,
> - "%s: missing direction_input() operation and line is output\n",
> - __func__);
> - return -EIO;
> + } else if (guard.gc->get_direction) {
> + ret = guard.gc->get_direction(guard.gc,
> + gpio_chip_hwgpio(desc));
> + if (ret < 0)
> + return ret;
> +
> + if (ret != GPIO_LINE_DIRECTION_IN) {
> + gpiod_warn(desc,
> + "%s: missing direction_input() operation and line is output\n",
> + __func__);
> + return -EIO;
> + }
> }
> if (ret == 0) {
> clear_bit(FLAG_IS_OUT, &desc->flags);
> @@ -2771,12 +2779,18 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
> gpio_chip_hwgpio(desc), val);
> } else {
> /* Check that we are in output mode if we can */
> - if (guard.gc->get_direction &&
> - guard.gc->get_direction(guard.gc, gpio_chip_hwgpio(desc))) {
> - gpiod_warn(desc,
> - "%s: missing direction_output() operation\n",
> - __func__);
> - return -EIO;
> + if (guard.gc->get_direction) {
> + ret = guard.gc->get_direction(guard.gc,
> + gpio_chip_hwgpio(desc));
> + if (ret < 0)
> + return ret;
> +
> + if (ret != GPIO_LINE_DIRECTION_OUT) {
> + gpiod_warn(desc,
> + "%s: missing direction_output() operation\n",
> + __func__);
> + return -EIO;
> + }
> }
> /*
> * If we can't actively set the direction, we are some
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists