Re: [PATCH v1] core: Relax gpiod_chip_open() for symbolic links

[Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>]

pt., 7 lut 2020 o 11:30 Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> napisał(a):
>
> On Fri, Feb 07, 2020 at 11:13:43AM +0100, Bartosz Golaszewski wrote:
> > czw., 6 lut 2020 o 19:14 Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> napisał(a):
> > >
> > > User may ask device helper tool, for example, udev, to create a specific
> > > symbolic link to a device node. GPIO chip character device node is not
> > > exceptional. However, libgpiod in the commit d9b1c1f14c6b
> > > ("core: harden gpiod_chip_open()") went way too far in the hardening device
> > > node check.
> > >
> > > Relax that hardening for symbolic link to fix the regression.
> > >
> > > Reproducer:
> > >
> > >   % gpioinfo /dev/gpiochip5
> > >   gpiochip5 - 16 lines:
> > >       line   0:  "MUX33_DIR" "uart1-rx-oe" output active-high [used]
> > >       ...
> > >
> > >   % ln -sf /dev/gpiochip5 /dev/MyGPIO_5
> > >
> > >   % gpioinfo /dev/MyGPIO_5
> > >   gpioinfo: looking up chip /dev/MyGPIO_5: Inappropriate ioctl for device
> > >
> > > Link: https://stackoverflow.com/questions/60057494/gpio-issue-with-sym-link
> > > Fixes: d9b1c1f14c6b ("core: harden gpiod_chip_open()")
> > > Cc: Bartosz Golaszewski <bartekgola@xxxxxxxxx>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> >
> > Hi Andy,
> >
> > thanks for this - it makes perfect sense. One nit though: could you
> > keep the includes ordered alphabetically?
>
> Probably not. The user space relies a lot on header ordering. And limits.h
> sounds like one needed to be included first in many cases. That's why I moved
> it to the top. I can do it if you insist, but I consider it wrong approach for
> the record.

Nah, if anything headers may rely on some preprocessor defines coming
before them, but the ordering should be of no importance.

>
> > Also: it would be great if
> > you could add a test case for this to tests/tests-chip.c.
>
> I will look at it if I can do quickly something.
>

If not, don't worry - I can add it later myself.

Bartosz

> For the record I have tested it on Intel Edison with real GPIO chips (so,
> that's how reproducer code appears in the commit message).
>
> Thanks for review!
>
> --
> With Best Regards,
> Andy Shevchenko
>
>