linux-kernel - Re: [PATCH v2 1/2] driver core: Move the "removable" attribute from USB to core

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] [day] [month] [year] [list]

Message-ID: <20210512233225.GA137103@rocinante.localdomain>
Date:   Thu, 13 May 2021 01:32:25 +0200
From:   Krzysztof Wilczyński <kw@...ux.com>
To:     Rajat Jain <rajatja@...gle.com>
Cc:     Rajat Jain <rajatxjain@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Alan Stern <stern@...land.harvard.edu>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pci <linux-pci@...r.kernel.org>,
        "open list:ULTRA-WIDEBAND (UWB) SUBSYSTEM:" 
        <linux-usb@...r.kernel.org>, Bjorn Helgaas <helgaas@...nel.org>,
        Jesse Barnes <jsbarnes@...gle.com>,
        Dmitry Torokhov <dtor@...gle.com>
Subject: Re: [PATCH v2 1/2] driver core: Move the "removable" attribute from
 USB to core

Hi Rajat,

> Posted a v3 of this patch here:
> https://lore.kernel.org/patchwork/patch/1428133/

I saw!  Thank you!

I also found the original conversation around the main idea that's
driving the work done here, as per:

  https://lore.kernel.org/lkml/20200601232542.GA473883@bjorn-Precision-5520/

This helped to fill-in some missing pieces, so to speak, I've bad.

[...]
> > > > @@ -2504,8 +2523,16 @@ static int device_add_attrs(struct device *dev)
> > > >                       goto err_remove_dev_online;
> > > >       }
> > > >
> > > > +     if (type && type->supports_removable) {
> > > > +             error = device_create_file(dev, &dev_attr_removable);
> > > > +             if (error)
> > > > +                     goto err_remove_dev_waiting_for_supplier;
> > > > +     }
> > > > +
> > > >       return 0;
> > >
> > One of my primary considerations was also that the existing UAPI for
> > the USB's "removable" attribute shouldn't be changed. Currently, it
> > exists for all USB devices, so I think the current code / check is OK.
[...]

We wouldn't change addition of this attribute to the USB devices - after
the lift to the device core it still has to work as before, as you say.

Just to clarify.   What I was wondering is whether we should add this new
sysfs object so that every device now would get this attribute going
forward, regardless of whether it would have the "type->supports_removable"
set.

We would then have a lot of devices with this attribute set to
"unknown", and then were the is an actual support it would then be
either "fixed" or "removable".

Having said that, Bjorn pointed out to me that this might be
not necessarily desirable.

We were also wondering if we should only set DEVICE_REMOVABLE for
devices known to be behind an external-facing port, and let everything
else be set to "unknown" (or whatever the default would be).

Krzysztof