| 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: <81f890fc-6688-42f0-9756-567efc8bb97a@samsung.com>
Date: Tue, 25 Feb 2025 11:13:39 +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>
Subject: Re: [PATCH 8/8] gpiolib: sanitize the return value of
gpio_chip::get_direction()
On 10.02.2025 11:52, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
>
> As per the API contract, the get_direction() callback can only
> return 0, 1 or a negative error number. Add a wrapper around the callback
> calls that filters out anything else.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
This patch landed in today's linux-next as commit e623c4303ed11
("gpiolib: sanitize the return value of gpio_chip::get_direction()"). It
introduced a lockdep warning from the gpiochip_get_direction() function.
IMHO it looks that gpiochip_add_data_with_key() lacks proper srcu
locking/annotation for the newly created gpio chip. Here is the log:
gpio gpiochip1: Static allocation of GPIO base is deprecated, use
dynamic allocation.
------------[ cut here ]------------
WARNING: CPU: 2 PID: 35 at drivers/gpio/gpiolib.c:349
gpiochip_get_direction+0x48/0x66
Modules linked in: cdns_usb_common roles cdns3_starfive
snd_soc_simple_card snd_soc_simple_card_utils phy_jh7110_dphy_rx
clk_starfive_jh7110_vout pcie_starfive(+) clk_starfive_jh7110_isp
jh7110_trng sfctemp dwmac_starfive stmmac_platform
spi_cadence_quadspi(+) clk_starfive_jh7110_stg stmmac
clk_starfive_jh7110_aon jh7110_pwmdac pcs_xpcs phy_jh7110_usb spi_pl022
phy_jh7110_pcie snd_soc_spdif_tx i2c_dev drm
drm_panel_orientation_quirks backlight dm_mod configfs ip_tables x_tables
CPU: 2 UID: 0 PID: 35 Comm: kworker/u18:0 Tainted: G W
6.14.0-rc4-next-20250225 #1054
Tainted: [W]=WARN
Hardware name: StarFive VisionFive 2 v1.2A (DT)
Workqueue: events_unbound deferred_probe_work_func
epc : gpiochip_get_direction+0x48/0x66
ra : gpiochip_get_direction+0x46/0x66
...
[<ffffffff805fc72c>] gpiochip_get_direction+0x48/0x66
[<ffffffff80603a14>] gpiochip_add_data_with_key+0x74a/0xde2
[<ffffffff806044e6>] devm_gpiochip_add_data_with_key+0x1e/0x5a
[<ffffffff805f8738>] jh7110_pinctrl_probe+0x298/0x3aa
[<ffffffff80731116>] platform_probe+0x4e/0x92
[<ffffffff8000c366>] really_probe+0x10a/0x2de
[<ffffffff8000c5e4>] __driver_probe_device.part.0+0xaa/0xe0
[<ffffffff8072ee34>] driver_probe_device+0x78/0xc4
[<ffffffff8072eee6>] __device_attach_driver+0x66/0xc6
[<ffffffff8072d0b0>] bus_for_each_drv+0x5c/0xb0
[<ffffffff8072f33e>] __device_attach+0x84/0x13c
[<ffffffff8072f55e>] device_initial_probe+0xe/0x16
[<ffffffff8072e002>] bus_probe_device+0x88/0x8a
[<ffffffff8072e516>] deferred_probe_work_func+0xd4/0xee
[<ffffffff80047b7e>] process_one_work+0x1d0/0x57a
[<ffffffff8004854e>] worker_thread+0x166/0x2cc
[<ffffffff80051568>] kthread+0xdc/0x1b4
[<ffffffff80bcb942>] ret_from_fork+0xe/0x18
irq event stamp: 17857
hardirqs last enabled at (17857): [<ffffffff80bca986>]
_raw_spin_unlock_irqrestore+0x4c/0x4e
hardirqs last disabled at (17856): [<ffffffff80bca73c>]
_raw_spin_lock_irqsave+0x5e/0x64
softirqs last enabled at (17322): [<ffffffff80adff1a>]
inet6_fill_ifla6_attrs+0x3d0/0x420
softirqs last disabled at (17320): [<ffffffff80adfefe>]
inet6_fill_ifla6_attrs+0x3b4/0x420
---[ end trace 0000000000000000 ]---
> ---
> drivers/gpio/gpiolib.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 683a03d237c0..7f2aca9f81a1 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -341,6 +341,22 @@ static int gpiochip_find_base_unlocked(u16 ngpio)
> }
> }
>
> +static int gpiochip_get_direction(struct gpio_chip *gc, unsigned int offset)
> +{
> + int ret;
> +
> + lockdep_assert_held(&gc->gpiodev->srcu);
> +
> + if (WARN_ON(!gc->get_direction))
> + return -EOPNOTSUPP;
> +
> + ret = gc->get_direction(gc, offset);
> + if (ret > 1)
> + ret = -EBADE;
> +
> + return ret;
> +}
> +
> /**
> * gpiod_get_direction - return the current direction of a GPIO
> * @desc: GPIO to get the direction of
> @@ -381,7 +397,7 @@ int gpiod_get_direction(struct gpio_desc *desc)
> if (!guard.gc->get_direction)
> return -ENOTSUPP;
>
> - ret = guard.gc->get_direction(guard.gc, offset);
> + ret = gpiochip_get_direction(guard.gc, offset);
> if (ret < 0)
> return ret;
>
> @@ -1057,7 +1073,7 @@ 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)) {
> - ret = gc->get_direction(gc, desc_index);
> + ret = gpiochip_get_direction(gc, desc_index);
> if (ret < 0)
> goto err_cleanup_desc_srcu;
>
> @@ -2770,8 +2786,7 @@ int gpiod_direction_input_nonotify(struct gpio_desc *desc)
> ret = gpiochip_direction_input(guard.gc,
> gpio_chip_hwgpio(desc));
> } else if (guard.gc->get_direction) {
> - ret = guard.gc->get_direction(guard.gc,
> - gpio_chip_hwgpio(desc));
> + ret = gpiochip_get_direction(guard.gc, gpio_chip_hwgpio(desc));
> if (ret < 0)
> return ret;
>
> @@ -2818,8 +2833,8 @@ static int gpiod_direction_output_raw_commit(struct gpio_desc *desc, int value)
> } else {
> /* Check that we are in output mode if we can */
> if (guard.gc->get_direction) {
> - ret = guard.gc->get_direction(guard.gc,
> - gpio_chip_hwgpio(desc));
> + ret = gpiochip_get_direction(guard.gc,
> + gpio_chip_hwgpio(desc));
> if (ret < 0)
> return ret;
>
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists