linux-kernel - Re: [PATCH v5 2/3] usb: serial: xr

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Dec 2020 09:58:40 +0100
From:   Johan Hovold <johan@...nel.org>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Johan Hovold <johan@...nel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        linux-usb <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        patong.mxl@...il.com,
        Mauro Carvalho Chehab <mchehab+huawei@...nel.org>,
        Angelo Dureghello <angelo.dureghello@...esys.com>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>
Subject: Re: [PATCH v5 2/3] usb: serial: xr_serial: Add gpiochip support

On Sat, Dec 12, 2020 at 01:03:32AM +0100, Linus Walleij wrote:
> On Thu, Dec 10, 2020 at 9:53 AM Johan Hovold <johan@...nel.org> wrote:
> > On Wed, Dec 09, 2020 at 05:25:32PM +0100, Linus Walleij wrote:
> 
> > I just replied to that thread, but to summarize, you can't rely on
> > having the sysfs code detect collisions since that will trigger a bunch
> > of nasty warnings and backtraces. We also don't want the sysfs interface
> > for a specific USB device to depend on probe order (only the first one
> > plugged in gets to use the line names). And adding line names now could
> > in fact be what breaks currently working scripts.
> 
> Yes the sysfs ABI is very volatile and easy to break.
> 
> As pointed out in the other reply, sysfs base GPIO number is all
> wibbly-wobbly on anything hot-pluggable so in a way I feel it
> is the right thing to disallow sysfs altogether on hotpluggable
> devices.

No, the gpio numbers will of course vary, but since gpiolib exports the
base number for the chip, a scripts can easily determine the right gpio
number as base + offset.

Having probe order break that by sometimes exporting the line using it's
name is what would be a problem.

> > Just did a super quick check and it seems libgpiod still assumes a flat
> > name space. For example, gpiod_chip_find_line() returns only the first
> > line found that matches a name. Shouldn't be impossible to extend, but
> > just want to make sure this flat namespace assumption hasn't been to
> > heavily relied upon.
> 
> The unique way to identify a GPIO is gpiochip instance (with
> topology from sysfs) and then a line number on that chip.
> This is done e.g. in the example tool
> tools/gpio/gpio-hammer.c
> 
> As you can see the tool doesn't use these line names.
>
> The line names are really like symbolic links or something.
> But they are indeed in a flat namespace so we should try to
> at least make them unique if it turns out people love to use
> these.

Not necessarily, they could be unique per chip as we're discussing here
with respect to hotpluggable controllers. We just can't have it both
ways.

> As it is now system designers mostly use device tree to assign
> line names and they try to make these unique because they don't
> like the nasty warnings from gpiolib.
>
> If I google for the phrase "Detected name collision for GPIO name"
> I just find the code, our discussions and some USB serial devices
> warning about this so far.
> 
> Maybe we should just make a patch to disallow it?

That would make it impossible to provide name lines on hotpluggable
controllers, which would be nice to support.

Johan