linux-kernel - Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

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]

Message-ID: <20160825125305.GB15567@kroah.com>
Date:   Thu, 25 Aug 2016 08:53:05 -0400
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Rafał Miłecki <zajec5@...il.com>
Cc:     Richard Purdie <rpurdie@...ys.net>,
        Jacek Anaszewski <j.anaszewski@...sung.com>,
        Felipe Balbi <balbi@...nel.org>,
        Peter Chen <hzpeterchen@...il.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        Rafał Miłecki <rafal@...ecki.pl>,
        Jonathan Corbet <corbet@....net>,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Matthias Brugger <mbrugger@...e.com>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Stephan Linz <linz@...pro.net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:LED SUBSYSTEM" <linux-leds@...r.kernel.org>
Subject: Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

On Thu, Aug 25, 2016 at 07:14:52AM +0200, Rafał Miłecki wrote:
> On 24 August 2016 at 23:04, Greg KH <gregkh@...uxfoundation.org> wrote:
> > On Wed, Aug 24, 2016 at 11:29:51AM +0200, Rafał Miłecki wrote:
> >> On 24 August 2016 at 11:22, Greg KH <gregkh@...uxfoundation.org> wrote:
> >> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> >> >> +static ssize_t ports_show(struct device *dev, struct device_attribute *attr,
> >> >> +                       char *buf)
> >> >> +{
> >> >> +     struct led_classdev *led_cdev = dev_get_drvdata(dev);
> >> >> +     struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
> >> >> +     struct usbport_trig_port *port;
> >> >> +     ssize_t ret = 0;
> >> >> +     int len;
> >> >> +
> >> >> +     list_for_each_entry(port, &usbport_data->ports, list) {
> >> >> +             len = sprintf(buf + ret, "%s\n", port->name);
> >> >> +             if (len >= 0)
> >> >> +                     ret += len;
> >> >> +     }
> >> >> +
> >> >> +     return ret;
> >> >> +}
> >> >
> >> > sysfs is "one value per file", here you are listing a bunch of things in
> >> > one sysfs file.  Please don't do that.
> >>
> >> OK. What do you think about creating "ports" subdirectory and creating
> >> file-per-port in it? Then I'd need to bring back something like
> >> "new_port" and "remove_port". Does it sound OK?
> >
> > Maybe, I don't know.  Why is "USB" somehow unique here?  Why isn't this
> > the same for PCI slots/ports?  pccard?  sdcard?  thunderbolt?
> 
> Good question. I would like to extend this USB port trigger in the
> future by reacting to USB activity. This involves playing with URBs
> and I believe that at that point it'd be getting too much USB specific
> to /rule them all/.

Oh that's never going to work, sorry.  USB "activity" happens deep in
USB host controller hardware, not up at the kernel level at all.  It's
just too fast, and would take up too much CPU to do it otherwise.  Heck,
even old UHCI 1.1 USB controllers did this.

USB "activity" happens all the time, look at a USB bus analyzer if you
want to see what goes on.  Teasing out what is "real data" and what is
just "normal bus activity" is impossible at the kernel level, and really
makes no sense at all (your led would just be "on" all the time.)

Are you sure you know how USB works?  Writing things like this makes me
really worry...

greg k-h