linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] gpio: fix SRCU bugs
@ 2024-02-14  8:44 Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 1/4] gpio: take the SRCU read lock in gpiod_hog() Bartosz Golaszewski
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14  8:44 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Here are four fixes to some bugs in recent SRCU changes. The first one fixes
an actual race condition. The other three just make lockdep happy.

v1 -> v2:
- use srcu_dereference() instead of rcu_dereference_protected() as
  advised by Paul
- add a patch using rcu_dereference_check(..., 1) in deprecated
  interfaces that return the address of the RCU-protected chip structure
  to external users (who shouldn't use it anyway but well...)
- pick up review tags for patches 1/4 and 2/4

Bartosz Golaszewski (4):
  gpio: take the SRCU read lock in gpiod_hog()
  gpio: cdev: use correct pointer accessors with SRCU
  gpio: use srcu_dereference() with SRCU-protected pointers
  gpio: don't let lockdep complain about inherently dangerous RCU usage

 drivers/gpio/gpiolib-cdev.c  | 25 ++++++++++++-------------
 drivers/gpio/gpiolib-sysfs.c |  5 +++--
 drivers/gpio/gpiolib.c       | 32 ++++++++++++++++++--------------
 drivers/gpio/gpiolib.h       |  3 ++-
 4 files changed, 35 insertions(+), 30 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 1/4] gpio: take the SRCU read lock in gpiod_hog()
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
@ 2024-02-14  8:44 ` Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU Bartosz Golaszewski
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14  8:44 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, kernel test robot

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

gpiod_hog() may be called without the gpio_device SRCU read lock taken
so we need to do it here as well. It's alright if someone else is
already holding the lock as SRCU read critical sections can be nested.

Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index f5434e559382..439d32d5aa38 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -4492,24 +4492,27 @@ EXPORT_SYMBOL_GPL(gpiod_get_index_optional);
 int gpiod_hog(struct gpio_desc *desc, const char *name,
 	      unsigned long lflags, enum gpiod_flags dflags)
 {
-	struct gpio_chip *gc;
+	struct gpio_device *gdev = desc->gdev;
 	struct gpio_desc *local_desc;
 	int hwnum;
 	int ret;
 
+	CLASS(gpio_chip_guard, guard)(desc);
+	if (!guard.gc)
+		return -ENODEV;
+
 	if (test_and_set_bit(FLAG_IS_HOGGED, &desc->flags))
 		return 0;
 
-	gc = gpiod_to_chip(desc);
 	hwnum = gpio_chip_hwgpio(desc);
 
-	local_desc = gpiochip_request_own_desc(gc, hwnum, name,
+	local_desc = gpiochip_request_own_desc(guard.gc, hwnum, name,
 					       lflags, dflags);
 	if (IS_ERR(local_desc)) {
 		clear_bit(FLAG_IS_HOGGED, &desc->flags);
 		ret = PTR_ERR(local_desc);
 		pr_err("requesting hog GPIO %s (chip %s, offset %d) failed, %d\n",
-		       name, gc->label, hwnum, ret);
+		       name, gdev->label, hwnum, ret);
 		return ret;
 	}
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 1/4] gpio: take the SRCU read lock in gpiod_hog() Bartosz Golaszewski
@ 2024-02-14  8:44 ` Bartosz Golaszewski
       [not found]   ` <CGME20240216103104eucas1p1fac9b939d4af1648d222963fbef59645@eucas1p1.samsung.com>
  2024-02-26 14:27   ` kernel test robot
  2024-02-14  8:44 ` [PATCH v2 3/4] gpio: use srcu_dereference() with SRCU-protected pointers Bartosz Golaszewski
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14  8:44 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, kernel test robot

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

We never dereference the chip pointer in character device code so we can
use the lighter rcu_access_pointer() helper. This also makes lockep
happier as it no longer complains about suspicious rcu_dereference()
usage.

Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpio/gpiolib-cdev.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
index 9323b357df43..85037fa4925e 100644
--- a/drivers/gpio/gpiolib-cdev.c
+++ b/drivers/gpio/gpiolib-cdev.c
@@ -206,7 +206,7 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
 
 	guard(srcu)(&lh->gdev->srcu);
 
-	if (!rcu_dereference(lh->gdev->chip))
+	if (!rcu_access_pointer(lh->gdev->chip))
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1521,7 +1521,7 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
 
 	guard(srcu)(&lr->gdev->srcu);
 
-	if (!rcu_dereference(lr->gdev->chip))
+	if (!rcu_access_pointer(lr->gdev->chip))
 		return -ENODEV;
 
 	switch (cmd) {
@@ -1552,7 +1552,7 @@ static __poll_t linereq_poll(struct file *file,
 
 	guard(srcu)(&lr->gdev->srcu);
 
-	if (!rcu_dereference(lr->gdev->chip))
+	if (!rcu_access_pointer(lr->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &lr->wait, wait);
@@ -1574,7 +1574,7 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
 
 	guard(srcu)(&lr->gdev->srcu);
 
-	if (!rcu_dereference(lr->gdev->chip))
+	if (!rcu_access_pointer(lr->gdev->chip))
 		return -ENODEV;
 
 	if (count < sizeof(le))
@@ -1875,7 +1875,7 @@ static __poll_t lineevent_poll(struct file *file,
 
 	guard(srcu)(&le->gdev->srcu);
 
-	if (!rcu_dereference(le->gdev->chip))
+	if (!rcu_access_pointer(le->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &le->wait, wait);
@@ -1913,7 +1913,7 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
 
 	guard(srcu)(&le->gdev->srcu);
 
-	if (!rcu_dereference(le->gdev->chip))
+	if (!rcu_access_pointer(le->gdev->chip))
 		return -ENODEV;
 
 	/*
@@ -1996,7 +1996,7 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
 
 	guard(srcu)(&le->gdev->srcu);
 
-	if (!rcu_dereference(le->gdev->chip))
+	if (!rcu_access_pointer(le->gdev->chip))
 		return -ENODEV;
 
 	/*
@@ -2510,7 +2510,7 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 	guard(srcu)(&gdev->srcu);
 
 	/* We fail any subsequent ioctl():s when the chip is gone */
-	if (!rcu_dereference(gdev->chip))
+	if (!rcu_access_pointer(gdev->chip))
 		return -ENODEV;
 
 	/* Fill in the struct and pass to userspace */
@@ -2595,7 +2595,7 @@ static __poll_t lineinfo_watch_poll(struct file *file,
 
 	guard(srcu)(&cdev->gdev->srcu);
 
-	if (!rcu_dereference(cdev->gdev->chip))
+	if (!rcu_access_pointer(cdev->gdev->chip))
 		return EPOLLHUP | EPOLLERR;
 
 	poll_wait(file, &cdev->wait, pollt);
@@ -2618,7 +2618,7 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
 
 	guard(srcu)(&cdev->gdev->srcu);
 
-	if (!rcu_dereference(cdev->gdev->chip))
+	if (!rcu_access_pointer(cdev->gdev->chip))
 		return -ENODEV;
 
 #ifndef CONFIG_GPIO_CDEV_V1
@@ -2696,7 +2696,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
 	guard(srcu)(&gdev->srcu);
 
 	/* Fail on open if the backing gpiochip is gone */
-	if (!rcu_dereference(gdev->chip))
+	if (!rcu_access_pointer(gdev->chip))
 		return -ENODEV;
 
 	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
@@ -2796,8 +2796,7 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
 
 	guard(srcu)(&gdev->srcu);
 
-	gc = rcu_dereference(gdev->chip);
-	if (!gc)
+	if (!rcu_access_pointer(gdev->chip))
 		return -ENODEV;
 
 	chip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 3/4] gpio: use srcu_dereference() with SRCU-protected pointers
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 1/4] gpio: take the SRCU read lock in gpiod_hog() Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU Bartosz Golaszewski
@ 2024-02-14  8:44 ` Bartosz Golaszewski
  2024-02-14  8:44 ` [PATCH v2 4/4] gpio: don't let lockdep complain about inherently dangerous RCU usage Bartosz Golaszewski
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14  8:44 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, kernel test robot

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Lockdep with CONFIG_PROVE_RCU enabled reports false positives about
suspicious rcu_dereference() usage. Let's silence it by using
srcu_dereference() which is the correct helper with SRCU.

Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib-sysfs.c |  5 +++--
 drivers/gpio/gpiolib.c       | 16 ++++++++--------
 drivers/gpio/gpiolib.h       |  3 ++-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpio/gpiolib-sysfs.c b/drivers/gpio/gpiolib-sysfs.c
index 6285fa5afbb1..71ba2a774197 100644
--- a/drivers/gpio/gpiolib-sysfs.c
+++ b/drivers/gpio/gpiolib-sysfs.c
@@ -14,6 +14,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/string.h>
+#include <linux/srcu.h>
 #include <linux/sysfs.h>
 #include <linux/types.h>
 
@@ -756,7 +757,7 @@ int gpiochip_sysfs_register(struct gpio_device *gdev)
 
 	guard(srcu)(&gdev->srcu);
 
-	chip = rcu_dereference(gdev->chip);
+	chip = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!chip)
 		return -ENODEV;
 
@@ -800,7 +801,7 @@ void gpiochip_sysfs_unregister(struct gpio_device *gdev)
 
 	guard(srcu)(&gdev->srcu);
 
-	chip = rcu_dereference(gdev->chip);
+	chip = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (chip)
 		return;
 
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 439d32d5aa38..b095f475805f 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -109,7 +109,7 @@ const char *gpiod_get_label(struct gpio_desc *desc)
 		return "interrupt";
 
 	return test_bit(FLAG_REQUESTED, &flags) ?
-			rcu_dereference(desc->label) : NULL;
+			srcu_dereference(desc->label, &desc->srcu) : NULL;
 }
 
 static int desc_set_label(struct gpio_desc *desc, const char *label)
@@ -447,7 +447,7 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
 		guard(srcu)(&gdev->srcu);
 
-		gc = rcu_dereference(gdev->chip);
+		gc = srcu_dereference(gdev->chip, &gdev->srcu);
 		if (!gc)
 			continue;
 
@@ -1190,7 +1190,7 @@ struct gpio_device *gpio_device_find(void *data,
 				 srcu_read_lock_held(&gpio_devices_srcu)) {
 		guard(srcu)(&gdev->srcu);
 
-		gc = rcu_dereference(gdev->chip);
+		gc = srcu_dereference(gdev->chip, &gdev->srcu);
 
 		if (gc && match(gc, data))
 			return gpio_device_get(gdev);
@@ -2978,7 +2978,7 @@ static int gpiod_get_raw_value_commit(const struct gpio_desc *desc)
 
 	guard(srcu)(&gdev->srcu);
 
-	gc = rcu_dereference(gdev->chip);
+	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc)
 		return -ENODEV;
 
@@ -3012,7 +3012,7 @@ static bool gpio_device_chip_cmp(struct gpio_device *gdev, struct gpio_chip *gc)
 {
 	guard(srcu)(&gdev->srcu);
 
-	return gc == rcu_dereference(gdev->chip);
+	return gc == srcu_dereference(gdev->chip, &gdev->srcu);
 }
 
 int gpiod_get_array_value_complex(bool raw, bool can_sleep,
@@ -3593,7 +3593,7 @@ int gpiod_to_irq(const struct gpio_desc *desc)
 	gdev = desc->gdev;
 	/* FIXME Cannot use gpio_chip_guard due to const desc. */
 	guard(srcu)(&gdev->srcu);
-	gc = rcu_dereference(gdev->chip);
+	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc)
 		return -ENODEV;
 
@@ -4787,7 +4787,7 @@ static void gpiolib_dbg_show(struct seq_file *s, struct gpio_device *gdev)
 
 	guard(srcu)(&gdev->srcu);
 
-	gc = rcu_dereference(gdev->chip);
+	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc) {
 		seq_puts(s, "Underlying GPIO chip is gone\n");
 		return;
@@ -4872,7 +4872,7 @@ static int gpiolib_seq_show(struct seq_file *s, void *v)
 
 	guard(srcu)(&gdev->srcu);
 
-	gc = rcu_dereference(gdev->chip);
+	gc = srcu_dereference(gdev->chip, &gdev->srcu);
 	if (!gc) {
 		seq_printf(s, "%s%s: (dangling chip)",
 			   priv->newline ? "\n" : "",
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 07443d26cbca..ada36aa0f81a 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -202,7 +202,8 @@ DEFINE_CLASS(gpio_chip_guard,
 
 		_guard.gdev = desc->gdev;
 		_guard.idx = srcu_read_lock(&_guard.gdev->srcu);
-		_guard.gc = rcu_dereference(_guard.gdev->chip);
+		_guard.gc = srcu_dereference(_guard.gdev->chip,
+					     &_guard.gdev->srcu);
 
 		_guard;
 	     }),
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 4/4] gpio: don't let lockdep complain about inherently dangerous RCU usage
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
                   ` (2 preceding siblings ...)
  2024-02-14  8:44 ` [PATCH v2 3/4] gpio: use srcu_dereference() with SRCU-protected pointers Bartosz Golaszewski
@ 2024-02-14  8:44 ` Bartosz Golaszewski
  2024-02-14 12:53 ` [PATCH v2 0/4] gpio: fix SRCU bugs Andy Shevchenko
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14  8:44 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, kernel test robot

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

There are two legacy, deprecated functions - gpiod_to_chip() and
gpio_device_get_chip() - that still have users in tree. They return the
address of the SRCU-protected chip outside of the read-only critical
sections. They are inherently dangerous and the users should convert to
safer alternatives. Let's explicitly silence lockdep warnings by using
rcu_dereference_check(ptr, 1). While at it: reuse
gpio_device_get_chip() in gpiod_to_chip().

Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 drivers/gpio/gpiolib.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index b095f475805f..02be0ba1a402 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -221,7 +221,8 @@ struct gpio_chip *gpiod_to_chip(const struct gpio_desc *desc)
 {
 	if (!desc)
 		return NULL;
-	return rcu_dereference(desc->gdev->chip);
+
+	return gpio_device_get_chip(desc->gdev);
 }
 EXPORT_SYMBOL_GPL(gpiod_to_chip);
 
@@ -291,7 +292,7 @@ EXPORT_SYMBOL(gpio_device_get_label);
  */
 struct gpio_chip *gpio_device_get_chip(struct gpio_device *gdev)
 {
-	return rcu_dereference(gdev->chip);
+	return rcu_dereference_check(gdev->chip, 1);
 }
 EXPORT_SYMBOL_GPL(gpio_device_get_chip);
 
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
                   ` (3 preceding siblings ...)
  2024-02-14  8:44 ` [PATCH v2 4/4] gpio: don't let lockdep complain about inherently dangerous RCU usage Bartosz Golaszewski
@ 2024-02-14 12:53 ` Andy Shevchenko
  2024-02-14 13:02   ` Bartosz Golaszewski
  2024-02-14 13:17 ` Mark Brown
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Andy Shevchenko @ 2024-02-14 12:53 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> an actual race condition. The other three just make lockdep happy.

Same comment here, can we simply redo this work so we won't have tons of fixes
on top of the nice RCU rework?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14 12:53 ` [PATCH v2 0/4] gpio: fix SRCU bugs Andy Shevchenko
@ 2024-02-14 13:02   ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14 13:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 1:53 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> > an actual race condition. The other three just make lockdep happy.
>
> Same comment here, can we simply redo this work so we won't have tons of fixes
> on top of the nice RCU rework?
>

The rework is clearly a new development - not meant for backporting. I
don't see any benefit from rebasing. These are normal fixes for a big
rework.

Bart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
                   ` (4 preceding siblings ...)
  2024-02-14 12:53 ` [PATCH v2 0/4] gpio: fix SRCU bugs Andy Shevchenko
@ 2024-02-14 13:17 ` Mark Brown
  2024-02-14 13:22   ` Bartosz Golaszewski
  2024-02-14 18:44 ` Paul E. McKenney
  2024-02-15  7:43 ` Bartosz Golaszewski
  7 siblings, 1 reply; 18+ messages in thread
From: Mark Brown @ 2024-02-14 13:17 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> an actual race condition. The other three just make lockdep happy.

This doesn't fix the issue I reported yesterday when applied on top of
today's next:

   https://lava.sirena.org.uk/scheduler/job/585469

[    1.995518] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000078

...

[    2.176162] Call trace:
[    2.178610]  check_init_srcu_struct+0x1c/0xa0
[    2.182974]  synchronize_srcu+0x1c/0x100
[    2.186904]  gpiod_request_commit+0xec/0x1e0
[    2.191183]  gpiochip_request_own_desc+0x58/0x124
[    2.195894]  gpiod_hog+0x114/0x1b4
[    2.199305]  of_gpiochip_add+0x208/0x370
[    2.203232]  gpiochip_add_data_with_key+0x71c/0xf10
[    2.208117]  devm_gpiochip_add_data_with_key+0x30/0x7c
[    2.213261]  mxc_gpio_probe+0x208/0x4b0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14 13:17 ` Mark Brown
@ 2024-02-14 13:22   ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14 13:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 2:17 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> > an actual race condition. The other three just make lockdep happy.
>
> This doesn't fix the issue I reported yesterday when applied on top of
> today's next:
>
>    https://lava.sirena.org.uk/scheduler/job/585469
>
> [    1.995518] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000078
>
> ...
>
> [    2.176162] Call trace:
> [    2.178610]  check_init_srcu_struct+0x1c/0xa0
> [    2.182974]  synchronize_srcu+0x1c/0x100
> [    2.186904]  gpiod_request_commit+0xec/0x1e0
> [    2.191183]  gpiochip_request_own_desc+0x58/0x124
> [    2.195894]  gpiod_hog+0x114/0x1b4
> [    2.199305]  of_gpiochip_add+0x208/0x370
> [    2.203232]  gpiochip_add_data_with_key+0x71c/0xf10
> [    2.208117]  devm_gpiochip_add_data_with_key+0x30/0x7c
> [    2.213261]  mxc_gpio_probe+0x208/0x4b0

No, the fix for this issue is in my tree as commit ba5c5effe02c
("gpio: initialize descriptor SRCU structure before adding OF-based
chips"). These are mostly fixes for lockdep warnings.

Bart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
                   ` (5 preceding siblings ...)
  2024-02-14 13:17 ` Mark Brown
@ 2024-02-14 18:44 ` Paul E. McKenney
  2024-02-14 19:08   ` Bartosz Golaszewski
  2024-02-15  7:43 ` Bartosz Golaszewski
  7 siblings, 1 reply; 18+ messages in thread
From: Paul E. McKenney @ 2024-02-14 18:44 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Andy Shevchenko, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> 
> Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> an actual race condition. The other three just make lockdep happy.

For 1/4-3/4:

Acked-by: Paul E. McKenney <paulmck@kernel.org>

For 4/4, you are playing with fire, but I will assume that you know what
you are doing.  ;-)

							Thanx, Paul

> v1 -> v2:
> - use srcu_dereference() instead of rcu_dereference_protected() as
>   advised by Paul
> - add a patch using rcu_dereference_check(..., 1) in deprecated
>   interfaces that return the address of the RCU-protected chip structure
>   to external users (who shouldn't use it anyway but well...)
> - pick up review tags for patches 1/4 and 2/4
> 
> Bartosz Golaszewski (4):
>   gpio: take the SRCU read lock in gpiod_hog()
>   gpio: cdev: use correct pointer accessors with SRCU
>   gpio: use srcu_dereference() with SRCU-protected pointers
>   gpio: don't let lockdep complain about inherently dangerous RCU usage
> 
>  drivers/gpio/gpiolib-cdev.c  | 25 ++++++++++++-------------
>  drivers/gpio/gpiolib-sysfs.c |  5 +++--
>  drivers/gpio/gpiolib.c       | 32 ++++++++++++++++++--------------
>  drivers/gpio/gpiolib.h       |  3 ++-
>  4 files changed, 35 insertions(+), 30 deletions(-)
> 
> -- 
> 2.40.1
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14 18:44 ` Paul E. McKenney
@ 2024-02-14 19:08   ` Bartosz Golaszewski
  2024-02-14 20:44     ` Paul E. McKenney
  2024-02-21 21:45     ` Linus Walleij
  0 siblings, 2 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-14 19:08 UTC (permalink / raw)
  To: paulmck
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Andy Shevchenko, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 7:44 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> >
> > Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> > an actual race condition. The other three just make lockdep happy.
>
> For 1/4-3/4:
>
> Acked-by: Paul E. McKenney <paulmck@kernel.org>
>
> For 4/4, you are playing with fire, but I will assume that you know what
> you are doing.  ;-)
>

Up until this rework, this gdev->chip pointer could go from under any
user at any point. Now we have this gpio_device wrapper that provides
an entry point to using the chip safely while protected by the SRCU
read lock. Anyone who is still accessing gpio_chip directly (and not
being the GPIO provider themselves) is asking for trouble. There's
however no point in spamming lockdep splats in this case. I may end up
adding a warning to these routines.

Unfortunately, it's hard to fix 15 years of technical debt. :(

Thanks for the Acks.
Bartosz

[snip]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14 19:08   ` Bartosz Golaszewski
@ 2024-02-14 20:44     ` Paul E. McKenney
  2024-02-21 21:45     ` Linus Walleij
  1 sibling, 0 replies; 18+ messages in thread
From: Paul E. McKenney @ 2024-02-14 20:44 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Andy Shevchenko, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 08:08:33PM +0100, Bartosz Golaszewski wrote:
> On Wed, Feb 14, 2024 at 7:44 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> >
> > On Wed, Feb 14, 2024 at 09:44:15AM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> > > an actual race condition. The other three just make lockdep happy.
> >
> > For 1/4-3/4:
> >
> > Acked-by: Paul E. McKenney <paulmck@kernel.org>
> >
> > For 4/4, you are playing with fire, but I will assume that you know what
> > you are doing.  ;-)
> 
> Up until this rework, this gdev->chip pointer could go from under any
> user at any point. Now we have this gpio_device wrapper that provides
> an entry point to using the chip safely while protected by the SRCU
> read lock. Anyone who is still accessing gpio_chip directly (and not
> being the GPIO provider themselves) is asking for trouble. There's
> however no point in spamming lockdep splats in this case. I may end up
> adding a warning to these routines.
> 
> Unfortunately, it's hard to fix 15 years of technical debt. :(

Indeed, life is sometimes hard!

							Thanx, Paul

> Thanks for the Acks.
> Bartosz
> 
> [snip]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
                   ` (6 preceding siblings ...)
  2024-02-14 18:44 ` Paul E. McKenney
@ 2024-02-15  7:43 ` Bartosz Golaszewski
  7 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-15  7:43 UTC (permalink / raw)
  To: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 9:44 AM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Here are four fixes to some bugs in recent SRCU changes. The first one fixes
> an actual race condition. The other three just make lockdep happy.
>
> v1 -> v2:
> - use srcu_dereference() instead of rcu_dereference_protected() as
>   advised by Paul
> - add a patch using rcu_dereference_check(..., 1) in deprecated
>   interfaces that return the address of the RCU-protected chip structure
>   to external users (who shouldn't use it anyway but well...)
> - pick up review tags for patches 1/4 and 2/4
>
> Bartosz Golaszewski (4):
>   gpio: take the SRCU read lock in gpiod_hog()
>   gpio: cdev: use correct pointer accessors with SRCU
>   gpio: use srcu_dereference() with SRCU-protected pointers
>   gpio: don't let lockdep complain about inherently dangerous RCU usage
>
>  drivers/gpio/gpiolib-cdev.c  | 25 ++++++++++++-------------
>  drivers/gpio/gpiolib-sysfs.c |  5 +++--
>  drivers/gpio/gpiolib.c       | 32 ++++++++++++++++++--------------
>  drivers/gpio/gpiolib.h       |  3 ++-
>  4 files changed, 35 insertions(+), 30 deletions(-)
>
> --
> 2.40.1
>

Series queued.

Bart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
       [not found]   ` <CGME20240216103104eucas1p1fac9b939d4af1648d222963fbef59645@eucas1p1.samsung.com>
@ 2024-02-16 10:31     ` Marek Szyprowski
  2024-02-16 10:50       ` Bartosz Golaszewski
  0 siblings, 1 reply; 18+ messages in thread
From: Marek Szyprowski @ 2024-02-16 10:31 UTC (permalink / raw)
  To: Bartosz Golaszewski, Linus Walleij, Kent Gibson, Alex Elder,
	Geert Uytterhoeven, Paul E . McKenney, Andy Shevchenko,
	Wolfram Sang, Mark Brown, Dan Carpenter
  Cc: linux-gpio, linux-kernel, Bartosz Golaszewski, kernel test robot

On 14.02.2024 09:44, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> We never dereference the chip pointer in character device code so we can
> use the lighter rcu_access_pointer() helper. This also makes lockep
> happier as it no longer complains about suspicious rcu_dereference()
> usage.
>
> Fixes: d83cee3d2bb1 ("gpio: protect the pointer to gpio_chip in gpio_device with SRCU")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202402122234.d85cca9b-lkp@intel.com
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>   drivers/gpio/gpiolib-cdev.c | 25 ++++++++++++-------------
>   1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c
> index 9323b357df43..85037fa4925e 100644
> --- a/drivers/gpio/gpiolib-cdev.c
> +++ b/drivers/gpio/gpiolib-cdev.c
> @@ -206,7 +206,7 @@ static long linehandle_ioctl(struct file *file, unsigned int cmd,
>   
>   	guard(srcu)(&lh->gdev->srcu);
>   
> -	if (!rcu_dereference(lh->gdev->chip))
> +	if (!rcu_access_pointer(lh->gdev->chip))
>   		return -ENODEV;
>   
>   	switch (cmd) {
> @@ -1521,7 +1521,7 @@ static long linereq_ioctl(struct file *file, unsigned int cmd,
>   
>   	guard(srcu)(&lr->gdev->srcu);
>   
> -	if (!rcu_dereference(lr->gdev->chip))
> +	if (!rcu_access_pointer(lr->gdev->chip))
>   		return -ENODEV;
>   
>   	switch (cmd) {
> @@ -1552,7 +1552,7 @@ static __poll_t linereq_poll(struct file *file,
>   
>   	guard(srcu)(&lr->gdev->srcu);
>   
> -	if (!rcu_dereference(lr->gdev->chip))
> +	if (!rcu_access_pointer(lr->gdev->chip))
>   		return EPOLLHUP | EPOLLERR;
>   
>   	poll_wait(file, &lr->wait, wait);
> @@ -1574,7 +1574,7 @@ static ssize_t linereq_read(struct file *file, char __user *buf,
>   
>   	guard(srcu)(&lr->gdev->srcu);
>   
> -	if (!rcu_dereference(lr->gdev->chip))
> +	if (!rcu_access_pointer(lr->gdev->chip))
>   		return -ENODEV;
>   
>   	if (count < sizeof(le))
> @@ -1875,7 +1875,7 @@ static __poll_t lineevent_poll(struct file *file,
>   
>   	guard(srcu)(&le->gdev->srcu);
>   
> -	if (!rcu_dereference(le->gdev->chip))
> +	if (!rcu_access_pointer(le->gdev->chip))
>   		return EPOLLHUP | EPOLLERR;
>   
>   	poll_wait(file, &le->wait, wait);
> @@ -1913,7 +1913,7 @@ static ssize_t lineevent_read(struct file *file, char __user *buf,
>   
>   	guard(srcu)(&le->gdev->srcu);
>   
> -	if (!rcu_dereference(le->gdev->chip))
> +	if (!rcu_access_pointer(le->gdev->chip))
>   		return -ENODEV;
>   
>   	/*
> @@ -1996,7 +1996,7 @@ static long lineevent_ioctl(struct file *file, unsigned int cmd,
>   
>   	guard(srcu)(&le->gdev->srcu);
>   
> -	if (!rcu_dereference(le->gdev->chip))
> +	if (!rcu_access_pointer(le->gdev->chip))
>   		return -ENODEV;
>   
>   	/*
> @@ -2510,7 +2510,7 @@ static long gpio_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>   	guard(srcu)(&gdev->srcu);
>   
>   	/* We fail any subsequent ioctl():s when the chip is gone */
> -	if (!rcu_dereference(gdev->chip))
> +	if (!rcu_access_pointer(gdev->chip))
>   		return -ENODEV;
>   
>   	/* Fill in the struct and pass to userspace */
> @@ -2595,7 +2595,7 @@ static __poll_t lineinfo_watch_poll(struct file *file,
>   
>   	guard(srcu)(&cdev->gdev->srcu);
>   
> -	if (!rcu_dereference(cdev->gdev->chip))
> +	if (!rcu_access_pointer(cdev->gdev->chip))
>   		return EPOLLHUP | EPOLLERR;
>   
>   	poll_wait(file, &cdev->wait, pollt);
> @@ -2618,7 +2618,7 @@ static ssize_t lineinfo_watch_read(struct file *file, char __user *buf,
>   
>   	guard(srcu)(&cdev->gdev->srcu);
>   
> -	if (!rcu_dereference(cdev->gdev->chip))
> +	if (!rcu_access_pointer(cdev->gdev->chip))
>   		return -ENODEV;
>   
>   #ifndef CONFIG_GPIO_CDEV_V1
> @@ -2696,7 +2696,7 @@ static int gpio_chrdev_open(struct inode *inode, struct file *file)
>   	guard(srcu)(&gdev->srcu);
>   
>   	/* Fail on open if the backing gpiochip is gone */
> -	if (!rcu_dereference(gdev->chip))
> +	if (!rcu_access_pointer(gdev->chip))
>   		return -ENODEV;
>   
>   	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
> @@ -2796,8 +2796,7 @@ int gpiolib_cdev_register(struct gpio_device *gdev, dev_t devt)
>   
>   	guard(srcu)(&gdev->srcu);
>   
> -	gc = rcu_dereference(gdev->chip);
> -	if (!gc)
> +	if (!rcu_access_pointer(gdev->chip))
>   		return -ENODEV;
>   
>   	chip_dbg(gc, "added GPIO chardev (%d:%d)\n", MAJOR(devt), gdev->id);

Here 'gc' is left uninitialized and nukes if GPIO DEBUGs are enabled. 
Here is an example (captured on today's linux-next):

8<--- cut here ---
Unable to handle kernel NULL pointer dereference at virtual address 
00000000 when read
[00000000] *pgd=00000000
Internal error: Oops: 5 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc4-next-20240216 #8096
Hardware name: Samsung Exynos (Flattened Device Tree)
PC is at gpiolib_cdev_register+0xd4/0x170
LR is at chainhash_table+0x784/0x20000
pc : [<c05dbe54>]    lr : [<c18bb74c>]    psr: 20000013
...
Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 4000404a  DAC: 00000051
Register r0 information: non-slab/vmalloc memory
Register r1 information: NULL pointer
Register r2 information: non-paged memory
Register r3 information: non-paged memory
Register r4 information: slab kmalloc-1k start c1e5f800 pointer offset 0 
size 1024
Register r5 information: NULL pointer
Register r6 information: non-paged memory
Register r7 information: slab kmalloc-1k start c1e5f800 pointer offset 
952 size 1024
Register r8 information: NULL pointer
Register r9 information: slab kmalloc-1k start c1e5f800 pointer offset 
960 size 1024
Register r10 information: non-paged memory
Register r11 information: non-slab/vmalloc memory
Register r12 information: NULL pointer
Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
Stack: (0xf082db90 to 0xf082e000)
...
  gpiolib_cdev_register from gpiochip_setup_dev+0x44/0xb0
  gpiochip_setup_dev from gpiochip_add_data_with_key+0x9ac/0xaac
  gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
  devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x938/0xb18
  samsung_pinctrl_probe from platform_probe+0x5c/0xb8
  platform_probe from really_probe+0xe0/0x400
  really_probe from __driver_probe_device+0x9c/0x1f4
  __driver_probe_device from driver_probe_device+0x30/0xc0
  driver_probe_device from __device_attach_driver+0xa8/0x120
  __device_attach_driver from bus_for_each_drv+0x80/0xcc
  bus_for_each_drv from __device_attach+0xac/0x1fc
  __device_attach from bus_probe_device+0x8c/0x90
  bus_probe_device from device_add+0x5d4/0x7fc
  device_add from of_platform_device_create_pdata+0x94/0xc4
  of_platform_device_create_pdata from of_platform_bus_create+0x1f8/0x4c0
  of_platform_bus_create from of_platform_bus_create+0x268/0x4c0
  of_platform_bus_create from of_platform_populate+0x80/0x110
  of_platform_populate from of_platform_default_populate_init+0xd4/0xec
  of_platform_default_populate_init from do_one_initcall+0x64/0x2fc
  do_one_initcall from kernel_init_freeable+0x1c4/0x228
  kernel_init_freeable from kernel_init+0x1c/0x12c
  kernel_init from ret_from_fork+0x14/0x28
Exception stack(0xf082dfb0 to 0xf082dff8)
...
---[ end trace 0000000000000000 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
---[ end Kernel panic - not syncing: Attempted to kill init! 
exitcode=0x0000000b ]---


Probably the easiest way to fix this issue is to replace chip_dbg with 
the following dev_dbg() call:

dev_dbg(&gdev->dev, "(%s): added GPIO chardev (%d:%d)\n", gdev->label, 
MAJOR(devt), gdev->id);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
  2024-02-16 10:31     ` Marek Szyprowski
@ 2024-02-16 10:50       ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-16 10:50 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Linus Walleij, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Paul E . McKenney, Andy Shevchenko, Wolfram Sang, Mark Brown,
	Dan Carpenter, linux-gpio, linux-kernel, Bartosz Golaszewski,
	kernel test robot

On Fri, Feb 16, 2024 at 11:31 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>

[snip]

>
> Here 'gc' is left uninitialized and nukes if GPIO DEBUGs are enabled.
> Here is an example (captured on today's linux-next):
>
> 8<--- cut here ---
> Unable to handle kernel NULL pointer dereference at virtual address
> 00000000 when read
> [00000000] *pgd=00000000
> Internal error: Oops: 5 [#1] PREEMPT SMP ARM
> Modules linked in:
> CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.8.0-rc4-next-20240216 #8096
> Hardware name: Samsung Exynos (Flattened Device Tree)
> PC is at gpiolib_cdev_register+0xd4/0x170
> LR is at chainhash_table+0x784/0x20000
> pc : [<c05dbe54>]    lr : [<c18bb74c>]    psr: 20000013
> ...
> Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 10c5387d  Table: 4000404a  DAC: 00000051
> Register r0 information: non-slab/vmalloc memory
> Register r1 information: NULL pointer
> Register r2 information: non-paged memory
> Register r3 information: non-paged memory
> Register r4 information: slab kmalloc-1k start c1e5f800 pointer offset 0
> size 1024
> Register r5 information: NULL pointer
> Register r6 information: non-paged memory
> Register r7 information: slab kmalloc-1k start c1e5f800 pointer offset
> 952 size 1024
> Register r8 information: NULL pointer
> Register r9 information: slab kmalloc-1k start c1e5f800 pointer offset
> 960 size 1024
> Register r10 information: non-paged memory
> Register r11 information: non-slab/vmalloc memory
> Register r12 information: NULL pointer
> Process swapper/0 (pid: 1, stack limit = 0x(ptrval))
> Stack: (0xf082db90 to 0xf082e000)
> ...
>   gpiolib_cdev_register from gpiochip_setup_dev+0x44/0xb0
>   gpiochip_setup_dev from gpiochip_add_data_with_key+0x9ac/0xaac
>   gpiochip_add_data_with_key from devm_gpiochip_add_data_with_key+0x20/0x5c
>   devm_gpiochip_add_data_with_key from samsung_pinctrl_probe+0x938/0xb18
>   samsung_pinctrl_probe from platform_probe+0x5c/0xb8
>   platform_probe from really_probe+0xe0/0x400
>   really_probe from __driver_probe_device+0x9c/0x1f4
>   __driver_probe_device from driver_probe_device+0x30/0xc0
>   driver_probe_device from __device_attach_driver+0xa8/0x120
>   __device_attach_driver from bus_for_each_drv+0x80/0xcc
>   bus_for_each_drv from __device_attach+0xac/0x1fc
>   __device_attach from bus_probe_device+0x8c/0x90
>   bus_probe_device from device_add+0x5d4/0x7fc
>   device_add from of_platform_device_create_pdata+0x94/0xc4
>   of_platform_device_create_pdata from of_platform_bus_create+0x1f8/0x4c0
>   of_platform_bus_create from of_platform_bus_create+0x268/0x4c0
>   of_platform_bus_create from of_platform_populate+0x80/0x110
>   of_platform_populate from of_platform_default_populate_init+0xd4/0xec
>   of_platform_default_populate_init from do_one_initcall+0x64/0x2fc
>   do_one_initcall from kernel_init_freeable+0x1c4/0x228
>   kernel_init_freeable from kernel_init+0x1c/0x12c
>   kernel_init from ret_from_fork+0x14/0x28
> Exception stack(0xf082dfb0 to 0xf082dff8)
> ...
> ---[ end trace 0000000000000000 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> ---[ end Kernel panic - not syncing: Attempted to kill init!
> exitcode=0x0000000b ]---
>
>
> Probably the easiest way to fix this issue is to replace chip_dbg with
> the following dev_dbg() call:
>
> dev_dbg(&gdev->dev, "(%s): added GPIO chardev (%d:%d)\n", gdev->label,
> MAJOR(devt), gdev->id);
>

Thanks for the report. Surprisingly there are no warnings about that
with GCC. The best way is to use srcu_dereference() here and keep the
same log message. Patch is on the way.

Bart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/4] gpio: fix SRCU bugs
  2024-02-14 19:08   ` Bartosz Golaszewski
  2024-02-14 20:44     ` Paul E. McKenney
@ 2024-02-21 21:45     ` Linus Walleij
  1 sibling, 0 replies; 18+ messages in thread
From: Linus Walleij @ 2024-02-21 21:45 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: paulmck, Kent Gibson, Alex Elder, Geert Uytterhoeven,
	Andy Shevchenko, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-gpio, linux-kernel, Bartosz Golaszewski

On Wed, Feb 14, 2024 at 8:08 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> Unfortunately, it's hard to fix 15 years of technical debt. :(

If it's any consolation I didn't create this one technical debt on purpose.

The actual mistake creating it goes something like: /dev/gpiochipN files are
nice and we can clean up after a file handle is closed or terminated,
I wonder why people insist on using sysfs for so many things.

All the file semantics of handles going away by being used etc, that's
why sysfs is good. I was too inexperienced to understand that, or I would
have paid more attention...

Yet I think we ended up in a reasonable place.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
  2024-02-14  8:44 ` [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU Bartosz Golaszewski
       [not found]   ` <CGME20240216103104eucas1p1fac9b939d4af1648d222963fbef59645@eucas1p1.samsung.com>
@ 2024-02-26 14:27   ` kernel test robot
  2024-02-27 16:44     ` Bartosz Golaszewski
  1 sibling, 1 reply; 18+ messages in thread
From: kernel test robot @ 2024-02-26 14:27 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: oe-lkp, lkp, kernel test robot, Linus Walleij, linux-gpio,
	Kent Gibson, Alex Elder, Geert Uytterhoeven, Paul E . McKenney,
	Andy Shevchenko, Wolfram Sang, Mark Brown, Dan Carpenter,
	linux-kernel, Bartosz Golaszewski



Hello,

kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on:

commit: ef4aabaa372144b2bfa9c4ddc83255831cff9ebd ("[PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU")
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-take-the-SRCU-read-lock-in-gpiod_hog/20240214-164752
base: https://git.kernel.org/cgit/linux/kernel/git/brgl/linux.git gpio/for-next
patch link: https://lore.kernel.org/all/20240214084419.6194-3-brgl@bgdev.pl/
patch subject: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU

in testcase: boot

compiler: clang-17
test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G

(please refer to attached dmesg/kmsg for entire log/backtrace)


+----------------------------------------------------------------------------------+------------+------------+
|                                                                                  | 5c3cf44c4e | ef4aabaa37 |
+----------------------------------------------------------------------------------+------------+------------+
| BUG:unable_to_handle_page_fault_for_address                                      | 0          | 14         |
| Oops:#[##]                                                                       | 0          | 14         |
| EIP:gpiolib_cdev_register                                                        | 0          | 14         |
| Kernel_panic-not_syncing:Fatal_exception                                         | 0          | 14         |
+----------------------------------------------------------------------------------+------------+------------+


If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202402262225.50983057-lkp@intel.com



[   57.990853][    T1] gpiochip_find_base_unlocked: found new base at 512
[   57.991570][    T1]
[   57.991875][    T1] =============================
[   57.992480][    T1] WARNING: suspicious RCU usage
[   57.992993][    T1] 6.8.0-rc4-00057-gef4aabaa3721 #1 Tainted: G                TN
[   57.993787][    T1] -----------------------------
[   57.994289][    T1] drivers/gpio/gpiolib.h:205 suspicious rcu_dereference_check() usage!
[   57.995135][    T1]
[   57.995135][    T1] other info that might help us debug this:
[   57.995135][    T1]
[   57.996198][    T1]
[   57.996198][    T1] rcu_scheduler_active = 2, debug_locks = 1
[   57.997073][    T1] 2 locks held by swapper/1:
[ 57.997559][ T1] #0: ea878e8c (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216) 
[ 57.998494][ T1] #1: eb682f3c (&gdev->srcu){.+.+}-{0:0}, at: srcu_lock_acquire (include/linux/srcu.h:115) 
[   57.999365][    T1]
[   57.999365][    T1] stack backtrace:
[   58.000005][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                TN 6.8.0-rc4-00057-gef4aabaa3721 #1 10e84cefff2a9bf1626b34c75cc7b6c846de4a3a
[   58.001317][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   58.002330][    T1] Call Trace:
[ 58.002690][ T1] dump_stack_lvl (lib/dump_stack.c:107) 
[ 58.003158][ T1] dump_stack (lib/dump_stack.c:113) 
[ 58.003631][ T1] lockdep_rcu_suspicious (kernel/locking/lockdep.c:6714) 
[ 58.006180][ T1] gpiod_hog (drivers/gpio/gpiolib.h:?) 
[ 58.006632][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799) 
[ 58.007165][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:829 drivers/gpio/gpiolib-of.c:1144) 
[ 58.007663][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:1013) 
[ 58.008278][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 58.008799][ T1] platform_probe (drivers/base/platform.c:1405) 
[ 58.009268][ T1] really_probe (drivers/base/dd.c:?) 
[ 58.009738][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 58.010265][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 58.010776][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 58.011258][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.011734][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 58.012234][ T1] driver_attach (drivers/base/dd.c:1233) 
[ 58.012692][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.013168][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 58.013652][ T1] driver_register (drivers/base/driver.c:246) 
[ 58.014177][ T1] __platform_driver_register (drivers/base/platform.c:867) 
[ 58.014730][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969) 
[ 58.015285][ T1] of_unittest_overlay (drivers/of/unittest.c:3217) 
[ 58.015804][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 58.016264][ T1] do_one_initcall (init/main.c:1236) 
[ 58.016755][ T1] ? dt_alloc_memory (drivers/of/unittest.c:4080) 
[ 58.017247][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.017766][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.018269][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.018802][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.019348][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.019864][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.020382][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.020900][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.021400][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.021931][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.022471][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.022979][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.023496][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.024000][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.024526][ T1] ? local_clock (arch/x86/include/asm/preempt.h:84 kernel/sched/clock.c:316) 
[ 58.024982][ T1] ? irqtime_account_irq (kernel/sched/cputime.c:?) 
[ 58.025553][ T1] ? irqtime_account_delta (include/linux/seqlock.h:444 include/linux/seqlock.h:516 include/linux/u64_stats_sync.h:151 include/linux/u64_stats_sync.h:187 kernel/sched/cputime.c:46) 
[ 58.026091][ T1] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:63) 
[ 58.026594][ T1] ? irqentry_exit (kernel/entry/common.c:?) 
[ 58.027070][ T1] ? common_interrupt (arch/x86/kernel/irq.c:247) 
[ 58.027567][ T1] ? asm_common_interrupt (arch/x86/entry/entry_32.S:640) 
[ 58.028119][ T1] ? strlen (arch/x86/lib/string_32.c:175) 
[ 58.028540][ T1] ? next_arg (lib/cmdline.c:273) 
[ 58.028991][ T1] ? parse_args (kernel/params.c:153) 
[ 58.029467][ T1] do_initcall_level (init/main.c:1297) 
[ 58.029957][ T1] ? rest_init (init/main.c:1433) 
[ 58.030398][ T1] do_initcalls (init/main.c:1311) 
[ 58.030846][ T1] ? rest_init (init/main.c:1433) 
[ 58.031299][ T1] do_basic_setup (init/main.c:1334) 
[ 58.031769][ T1] kernel_init_freeable (init/main.c:1555) 
[ 58.032283][ T1] kernel_init (init/main.c:1443) 
[ 58.032732][ T1] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 58.033192][ T1] ret_from_fork_asm (arch/x86/entry/entry_32.S:741) 
[ 58.033680][ T1] entry_INT80_32 (arch/x86/entry/entry_32.S:947) 
[   58.034672][    T1] gpio gpiochip0: Persistence not supported for GPIO 2
[   58.035366][    T1]
[   58.035669][    T1] =============================
[   58.036180][    T1] WARNING: suspicious RCU usage
[   58.036739][    T1] 6.8.0-rc4-00057-gef4aabaa3721 #1 Tainted: G                TN
[   58.037546][    T1] -----------------------------
[   58.038050][    T1] drivers/gpio/gpiolib.c:112 suspicious rcu_dereference_check() usage!
[   58.038893][    T1]
[   58.038893][    T1] other info that might help us debug this:
[   58.038893][    T1]
[   58.039950][    T1]
[   58.039950][    T1] rcu_scheduler_active = 2, debug_locks = 1
[   58.040837][    T1] 3 locks held by swapper/1:
[ 58.041318][ T1] #0: ea878e8c (&dev->mutex){....}-{3:3}, at: __driver_attach (drivers/base/dd.c:1216) 
[ 58.042234][ T1] #1: eb682f3c (&gdev->srcu){.+.+}-{0:0}, at: srcu_lock_acquire (include/linux/srcu.h:115) 
[ 58.043116][ T1] #2: eb6831dc (&desc->srcu){.+.+}-{0:0}, at: srcu_lock_acquire (include/linux/srcu.h:115) 
[   58.043992][    T1]
[   58.043992][    T1] stack backtrace:
[   58.044685][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                TN 6.8.0-rc4-00057-gef4aabaa3721 #1 10e84cefff2a9bf1626b34c75cc7b6c846de4a3a
[   58.046006][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[   58.047050][    T1] Call Trace:
[ 58.047417][ T1] dump_stack_lvl (lib/dump_stack.c:107) 
[ 58.047896][ T1] dump_stack (lib/dump_stack.c:113) 
[ 58.048430][ T1] lockdep_rcu_suspicious (kernel/locking/lockdep.c:6714) 
[ 58.048974][ T1] gpiod_get_label (drivers/gpio/gpiolib.c:?) 
[ 58.049450][ T1] gpiod_hog (drivers/gpio/gpiolib.c:4519) 
[ 58.049901][ T1] of_gpiochip_add_hog (drivers/gpio/gpiolib-of.c:799) 
[ 58.050433][ T1] of_gpiochip_add (drivers/gpio/gpiolib-of.c:829 drivers/gpio/gpiolib-of.c:1144) 
[ 58.050933][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:1013) 
[ 58.051503][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 58.052013][ T1] platform_probe (drivers/base/platform.c:1405) 
[ 58.052484][ T1] really_probe (drivers/base/dd.c:?) 
[ 58.052953][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 58.053480][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 58.054001][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 58.054482][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.054958][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 58.055451][ T1] driver_attach (drivers/base/dd.c:1233) 
[ 58.055908][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.056386][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 58.056871][ T1] driver_register (drivers/base/driver.c:246) 
[ 58.057348][ T1] __platform_driver_register (drivers/base/platform.c:867) 
[ 58.057900][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969) 
[ 58.058450][ T1] of_unittest_overlay (drivers/of/unittest.c:3217) 
[ 58.058973][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 58.059476][ T1] do_one_initcall (init/main.c:1236) 
[ 58.059967][ T1] ? dt_alloc_memory (drivers/of/unittest.c:4080) 
[ 58.060472][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.060994][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.061500][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.062037][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.062580][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.063084][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.063595][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.064122][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.064665][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.065201][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.065742][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.066248][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.066768][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.067271][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.067805][ T1] ? local_clock (arch/x86/include/asm/preempt.h:84 kernel/sched/clock.c:316) 
[ 58.068268][ T1] ? irqtime_account_irq (kernel/sched/cputime.c:?) 
[ 58.068795][ T1] ? irqtime_account_delta (include/linux/seqlock.h:444 include/linux/seqlock.h:516 include/linux/u64_stats_sync.h:151 include/linux/u64_stats_sync.h:187 kernel/sched/cputime.c:46) 
[ 58.069333][ T1] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:63) 
[ 58.069836][ T1] ? irqentry_exit (kernel/entry/common.c:?) 
[ 58.070341][ T1] ? common_interrupt (arch/x86/kernel/irq.c:247) 
[ 58.070842][ T1] ? asm_common_interrupt (arch/x86/entry/entry_32.S:640) 
[ 58.071388][ T1] ? strlen (arch/x86/lib/string_32.c:175) 
[ 58.071812][ T1] ? next_arg (lib/cmdline.c:273) 
[ 58.072278][ T1] ? parse_args (kernel/params.c:153) 
[ 58.072756][ T1] do_initcall_level (init/main.c:1297) 
[ 58.073247][ T1] ? rest_init (init/main.c:1433) 
[ 58.073693][ T1] do_initcalls (init/main.c:1311) 
[ 58.074142][ T1] ? rest_init (init/main.c:1433) 
[ 58.074585][ T1] do_basic_setup (init/main.c:1334) 
[ 58.075052][ T1] kernel_init_freeable (init/main.c:1555) 
[ 58.075573][ T1] kernel_init (init/main.c:1443) 
[ 58.076029][ T1] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 58.076498][ T1] ret_from_fork_asm (arch/x86/entry/entry_32.S:741) 
[ 58.076989][ T1] entry_INT80_32 (arch/x86/entry/entry_32.S:947) 
[   58.077544][    T1] gpio-514 (line-B-input): hogged as input
[   58.078433][    T1] BUG: unable to handle page fault for address: ffffffff
[   58.079116][    T1] #PF: supervisor read access in kernel mode
[   58.079703][    T1] #PF: error_code(0x0000) - not-present page
[   58.080316][    T1] *pde = 02e00067 *pte = 00000000
[   58.080829][    T1] Oops: 0000 [#1]
[   58.081210][    T1] CPU: 0 PID: 1 Comm: swapper Tainted: G                TN 6.8.0-rc4-00057-gef4aabaa3721 #1 10e84cefff2a9bf1626b34c75cc7b6c846de4a3a
[   58.082492][    T1] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
[ 58.083471][ T1] EIP: gpiolib_cdev_register (drivers/gpio/gpiolib-cdev.c:2802) 
[ 58.084015][ T1] Code: ec e8 7a 10 00 00 89 f8 89 da e8 c1 c8 a5 ff 89 f0 83 c4 0c 5e 5f 5b 5d 31 c9 31 d2 c3 8b 45 f0 c1 e8 14 ff b7 d8 01 00 00 50 <ff> 35 ff ff ff ff 68 7c de 5c c2 ff 35 03 00 00 00 68 3d ad 57 c2
All code
========
   0:	ec                   	in     (%dx),%al
   1:	e8 7a 10 00 00       	call   0x1080
   6:	89 f8                	mov    %edi,%eax
   8:	89 da                	mov    %ebx,%edx
   a:	e8 c1 c8 a5 ff       	call   0xffffffffffa5c8d0
   f:	89 f0                	mov    %esi,%eax
  11:	83 c4 0c             	add    $0xc,%esp
  14:	5e                   	pop    %rsi
  15:	5f                   	pop    %rdi
  16:	5b                   	pop    %rbx
  17:	5d                   	pop    %rbp
  18:	31 c9                	xor    %ecx,%ecx
  1a:	31 d2                	xor    %edx,%edx
  1c:	c3                   	ret
  1d:	8b 45 f0             	mov    -0x10(%rbp),%eax
  20:	c1 e8 14             	shr    $0x14,%eax
  23:	ff b7 d8 01 00 00    	push   0x1d8(%rdi)
  29:	50                   	push   %rax
  2a:*	ff 35 ff ff ff ff    	push   -0x1(%rip)        # 0x2f		<-- trapping instruction
  30:	68 7c de 5c c2       	push   $0xffffffffc25cde7c
  35:	ff 35 03 00 00 00    	push   0x3(%rip)        # 0x3e
  3b:	68 3d ad 57 c2       	push   $0xffffffffc257ad3d

Code starting with the faulting instruction
===========================================
   0:	ff 35 ff ff ff ff    	push   -0x1(%rip)        # 0x5
   6:	68 7c de 5c c2       	push   $0xffffffffc25cde7c
   b:	ff 35 03 00 00 00    	push   0x3(%rip)        # 0x14
  11:	68 3d ad 57 c2       	push   $0xffffffffc257ad3d
[   58.085813][    T1] EAX: 000000fe EBX: 00000000 ECX: 00000000 EDX: 00000000
[   58.086476][    T1] ESI: ffffffed EDI: eb682c00 EBP: c3e1baec ESP: c3e1bacc
[   58.087140][    T1] DS: 007b ES: 007b FS: 0000 GS: 0000 SS: 0068 EFLAGS: 00010202
[   58.087927][    T1] CR0: 80050033 CR2: ffffffff CR3: 02dfd000 CR4: 000406d0
[   58.088601][    T1] Call Trace:
[ 58.088950][ T1] ? __die_body (arch/x86/kernel/dumpstack.c:478 arch/x86/kernel/dumpstack.c:420) 
[ 58.089388][ T1] ? __die (arch/x86/kernel/dumpstack.c:434) 
[ 58.089790][ T1] ? page_fault_oops (arch/x86/mm/fault.c:703) 
[ 58.090279][ T1] ? kernelmode_fixup_or_oops (arch/x86/mm/fault.c:761) 
[ 58.090815][ T1] ? __bad_area_nosemaphore (arch/x86/mm/fault.c:817) 
[ 58.091344][ T1] ? bad_area_nosemaphore (arch/x86/mm/fault.c:866) 
[ 58.091846][ T1] ? do_kern_addr_fault (arch/x86/mm/fault.c:1226) 
[ 58.092347][ T1] ? exc_page_fault (arch/x86/mm/fault.c:?) 
[ 58.092822][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1520) 
[ 58.093407][ T1] ? handle_exception (arch/x86/entry/entry_32.S:1049) 
[ 58.093964][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1520) 
[ 58.094556][ T1] ? gpiolib_cdev_register (drivers/gpio/gpiolib-cdev.c:2802) 
[ 58.095076][ T1] ? pvclock_clocksource_read_nowd (arch/x86/mm/fault.c:1520) 
[ 58.095666][ T1] ? gpiolib_cdev_register (drivers/gpio/gpiolib-cdev.c:2802) 
[ 58.096187][ T1] gpiochip_setup_dev (drivers/gpio/gpiolib.c:740) 
[ 58.096665][ T1] gpiochip_add_data_with_key (drivers/gpio/gpiolib.c:1047) 
[ 58.099219][ T1] unittest_gpio_probe (drivers/of/unittest.c:1886) 
[ 58.099744][ T1] platform_probe (drivers/base/platform.c:1405) 
[ 58.100210][ T1] really_probe (drivers/base/dd.c:?) 
[ 58.100662][ T1] __driver_probe_device (drivers/base/dd.c:800) 
[ 58.101172][ T1] driver_probe_device (drivers/base/dd.c:830) 
[ 58.101667][ T1] __driver_attach (drivers/base/dd.c:1217) 
[ 58.102131][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.102587][ T1] bus_for_each_dev (drivers/base/bus.c:367) 
[ 58.103061][ T1] driver_attach (drivers/base/dd.c:1233) 
[ 58.103505][ T1] ? driver_attach (drivers/base/dd.c:1157) 
[ 58.103962][ T1] bus_add_driver (drivers/base/bus.c:674) 
[ 58.104431][ T1] driver_register (drivers/base/driver.c:246) 
[ 58.104902][ T1] __platform_driver_register (drivers/base/platform.c:867) 
[ 58.105443][ T1] of_unittest_overlay_gpio (drivers/of/unittest.c:1969) 
[ 58.105976][ T1] of_unittest_overlay (drivers/of/unittest.c:3217) 
[ 58.106475][ T1] of_unittest (drivers/of/unittest.c:4129) 
[ 58.106911][ T1] do_one_initcall (init/main.c:1236) 
[ 58.107384][ T1] ? dt_alloc_memory (drivers/of/unittest.c:4080) 
[ 58.107860][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.108367][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.108853][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.109364][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.109889][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.110377][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.110926][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.111428][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.111917][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.112441][ T1] ? rcu_read_lock_any_held (kernel/rcu/update.c:386) 
[ 58.112964][ T1] ? __lock_acquire (kernel/locking/lockdep.c:3762) 
[ 58.113450][ T1] ? kvm_sched_clock_read (arch/x86/kernel/kvmclock.c:91) 
[ 58.113964][ T1] ? sched_clock_noinstr (arch/x86/kernel/tsc.c:266) 
[ 58.114454][ T1] ? local_clock_noinstr (kernel/sched/clock.c:269 kernel/sched/clock.c:306) 
[ 58.114963][ T1] ? local_clock (arch/x86/include/asm/preempt.h:84 kernel/sched/clock.c:316) 
[ 58.115399][ T1] ? irqtime_account_irq (kernel/sched/cputime.c:?) 
[ 58.115903][ T1] ? irqtime_account_delta (include/linux/seqlock.h:444 include/linux/seqlock.h:516 include/linux/u64_stats_sync.h:151 include/linux/u64_stats_sync.h:187 kernel/sched/cputime.c:46) 
[ 58.116430][ T1] ? trace_hardirqs_on (kernel/trace/trace_preemptirq.c:63) 
[ 58.116929][ T1] ? irqentry_exit (kernel/entry/common.c:?) 
[ 58.117391][ T1] ? common_interrupt (arch/x86/kernel/irq.c:247) 
[ 58.117869][ T1] ? asm_common_interrupt (arch/x86/entry/entry_32.S:640) 
[ 58.118390][ T1] ? strlen (arch/x86/lib/string_32.c:175) 
[ 58.118799][ T1] ? next_arg (lib/cmdline.c:273) 
[ 58.119238][ T1] ? parse_args (kernel/params.c:153) 
[ 58.119689][ T1] do_initcall_level (init/main.c:1297) 
[ 58.120167][ T1] ? rest_init (init/main.c:1433) 
[ 58.120596][ T1] do_initcalls (init/main.c:1311) 
[ 58.121033][ T1] ? rest_init (init/main.c:1433) 
[ 58.121460][ T1] do_basic_setup (init/main.c:1334) 
[ 58.121908][ T1] kernel_init_freeable (init/main.c:1555) 
[ 58.122401][ T1] kernel_init (init/main.c:1443) 
[ 58.122873][ T1] ret_from_fork (arch/x86/kernel/process.c:153) 
[ 58.123330][ T1] ret_from_fork_asm (arch/x86/entry/entry_32.S:741) 
[ 58.123804][ T1] entry_INT80_32 (arch/x86/entry/entry_32.S:947) 
[   58.124281][    T1] Modules linked in:
[   58.124686][    T1] CR2: 00000000ffffffff
[   58.125108][    T1] ---[ end trace 0000000000000000 ]---
[ 58.125635][ T1] EIP: gpiolib_cdev_register (drivers/gpio/gpiolib-cdev.c:2802) 
[ 58.126171][ T1] Code: ec e8 7a 10 00 00 89 f8 89 da e8 c1 c8 a5 ff 89 f0 83 c4 0c 5e 5f 5b 5d 31 c9 31 d2 c3 8b 45 f0 c1 e8 14 ff b7 d8 01 00 00 50 <ff> 35 ff ff ff ff 68 7c de 5c c2 ff 35 03 00 00 00 68 3d ad 57 c2
All code
========
   0:	ec                   	in     (%dx),%al
   1:	e8 7a 10 00 00       	call   0x1080
   6:	89 f8                	mov    %edi,%eax
   8:	89 da                	mov    %ebx,%edx
   a:	e8 c1 c8 a5 ff       	call   0xffffffffffa5c8d0
   f:	89 f0                	mov    %esi,%eax
  11:	83 c4 0c             	add    $0xc,%esp
  14:	5e                   	pop    %rsi
  15:	5f                   	pop    %rdi
  16:	5b                   	pop    %rbx
  17:	5d                   	pop    %rbp
  18:	31 c9                	xor    %ecx,%ecx
  1a:	31 d2                	xor    %edx,%edx
  1c:	c3                   	ret
  1d:	8b 45 f0             	mov    -0x10(%rbp),%eax
  20:	c1 e8 14             	shr    $0x14,%eax
  23:	ff b7 d8 01 00 00    	push   0x1d8(%rdi)
  29:	50                   	push   %rax
  2a:*	ff 35 ff ff ff ff    	push   -0x1(%rip)        # 0x2f		<-- trapping instruction
  30:	68 7c de 5c c2       	push   $0xffffffffc25cde7c
  35:	ff 35 03 00 00 00    	push   0x3(%rip)        # 0x3e
  3b:	68 3d ad 57 c2       	push   $0xffffffffc257ad3d

Code starting with the faulting instruction
===========================================
   0:	ff 35 ff ff ff ff    	push   -0x1(%rip)        # 0x5
   6:	68 7c de 5c c2       	push   $0xffffffffc25cde7c
   b:	ff 35 03 00 00 00    	push   0x3(%rip)        # 0x14
  11:	68 3d ad 57 c2       	push   $0xffffffffc257ad3d


The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20240226/202402262225.50983057-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
  2024-02-26 14:27   ` kernel test robot
@ 2024-02-27 16:44     ` Bartosz Golaszewski
  0 siblings, 0 replies; 18+ messages in thread
From: Bartosz Golaszewski @ 2024-02-27 16:44 UTC (permalink / raw)
  To: kernel test robot
  Cc: oe-lkp, lkp, Linus Walleij, linux-gpio, Kent Gibson, Alex Elder,
	Geert Uytterhoeven, Paul E . McKenney, Andy Shevchenko,
	Wolfram Sang, Mark Brown, Dan Carpenter, linux-kernel,
	Bartosz Golaszewski

On Mon, Feb 26, 2024 at 3:28 PM kernel test robot <oliver.sang@intel.com> wrote:
>
>
>
> Hello,
>
> kernel test robot noticed "BUG:unable_to_handle_page_fault_for_address" on:
>
> commit: ef4aabaa372144b2bfa9c4ddc83255831cff9ebd ("[PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU")
> url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/gpio-take-the-SRCU-read-lock-in-gpiod_hog/20240214-164752
> base: https://git.kernel.org/cgit/linux/kernel/git/brgl/linux.git gpio/for-next
> patch link: https://lore.kernel.org/all/20240214084419.6194-3-brgl@bgdev.pl/
> patch subject: [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU
>
> in testcase: boot
>
> compiler: clang-17
> test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 16G
>
> (please refer to attached dmesg/kmsg for entire log/backtrace)
>

Please disregard this one, the issue is fixed in patch 3/4 which is
already in next.

Bart

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-02-27 16:44 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-14  8:44 [PATCH v2 0/4] gpio: fix SRCU bugs Bartosz Golaszewski
2024-02-14  8:44 ` [PATCH v2 1/4] gpio: take the SRCU read lock in gpiod_hog() Bartosz Golaszewski
2024-02-14  8:44 ` [PATCH v2 2/4] gpio: cdev: use correct pointer accessors with SRCU Bartosz Golaszewski
     [not found]   ` <CGME20240216103104eucas1p1fac9b939d4af1648d222963fbef59645@eucas1p1.samsung.com>
2024-02-16 10:31     ` Marek Szyprowski
2024-02-16 10:50       ` Bartosz Golaszewski
2024-02-26 14:27   ` kernel test robot
2024-02-27 16:44     ` Bartosz Golaszewski
2024-02-14  8:44 ` [PATCH v2 3/4] gpio: use srcu_dereference() with SRCU-protected pointers Bartosz Golaszewski
2024-02-14  8:44 ` [PATCH v2 4/4] gpio: don't let lockdep complain about inherently dangerous RCU usage Bartosz Golaszewski
2024-02-14 12:53 ` [PATCH v2 0/4] gpio: fix SRCU bugs Andy Shevchenko
2024-02-14 13:02   ` Bartosz Golaszewski
2024-02-14 13:17 ` Mark Brown
2024-02-14 13:22   ` Bartosz Golaszewski
2024-02-14 18:44 ` Paul E. McKenney
2024-02-14 19:08   ` Bartosz Golaszewski
2024-02-14 20:44     ` Paul E. McKenney
2024-02-21 21:45     ` Linus Walleij
2024-02-15  7:43 ` Bartosz Golaszewski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).