[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMRc=MfjQdFR_8ALGibxQnr5tzoHykCBpkBxjH78c5HuD43rBg@mail.gmail.com>
Date: Thu, 13 Jun 2024 13:22:58 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Linus Walleij <linus.walleij@...aro.org>, Jonathan Corbet <corbet@....net>,
"Rafael J . Wysocki" <rafael@...nel.org>, Arnd Bergmann <arnd@...db.de>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, linux-gpio@...r.kernel.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v8 2/2] misc: gpio-virtuser: new virtual testing driver
for the GPIO API
On Thu, Jun 13, 2024 at 12:02 PM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Thu, Jun 13, 2024 at 11:28:30AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > The GPIO subsystem used to have a serious problem with undefined behavior
> > and use-after-free bugs on hot-unplug of GPIO chips. This can be
> > considered a corner-case by some as most GPIO controllers are enabled
> > early in the boot process and live until the system goes down but most
> > GPIO drivers do allow unbind over sysfs, many are loadable modules that
> > can be (force) unloaded and there are also GPIO devices that can be
> > dynamically detached, for instance CP2112 which is a USB GPIO expender.
> >
> > Bugs can be triggered both from user-space as well as by in-kernel users.
> > We have the means of testing it from user-space via the character device
> > but the issues manifest themselves differently in the kernel.
> >
> > This is a proposition of adding a new virtual driver - a configurable
> > GPIO consumer that can be configured over configfs (similarly to
> > gpio-sim) or described on the device-tree.
> >
> > This driver is aimed as a helper in spotting any regressions in
> > hot-unplug handling in GPIOLIB.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> > ---
> > .../admin-guide/gpio/gpio-virtuser.rst | 176 ++
> > Documentation/admin-guide/gpio/index.rst | 1 +
>
> sysfs documentation needs to go in Documentation/ABI/ not in a random
> .rst file where the tools that check this will not catch it.
>
This is a testing driver, not representing real hardware. Do we hold
such modules to the same standard?
> > MAINTAINERS | 8 +
> > drivers/misc/Kconfig | 8 +
> > drivers/misc/Makefile | 1 +
> > drivers/misc/gpio-virtuser.c | 1790 +++++++++++++++++
>
> Why not put this in drivers/gpio/? Why misc?
>
Because it's quite... well "misc". It's not a GPIO chip provider
(drivers/gpio/ is for GPIO providers), it's only a GPIO consumer. It
also has an interface that doesn't fit any particular subsystem.
> > +Both attributes allow to read and set arrays of GPIO values. User must pass
> > +exactly the number of values that the array contains in the form of a string
> > +containing zeroes and ones representing inactive and active GPIO states
> > +respectively. In this example: ``echo 11 > values``.
>
> sysfs is "one value per file", so why are there multiple values here?
>
> If you want to just use this for testing, and want to put whatever you
> want in the files, just use debugfs, that's what it is there for, not
> sysfs.
>
Debugfs doesn't allow me to attach attributes to a particular device
which is what I want here.
Bart
> > +config GPIO_VIRTUSER
> > + tristate "GPIO Virtual User Testing Module"
> > + select CONFIGFS_FS
> > + select IRQ_WORK
> > + help
> > + This enables the configurable, configfs-based virtual GPIO consumer
> > + testing driver.
> > +
>
> module name?
>
> And you need more documentation here, I have no idea what this means
> when it shows up in a Kconfig help entry :(
>
> thanks,
>
> greg k-h
Powered by blists - more mailing lists