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

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

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.

> 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.

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