lkml.org 
[lkml]   [2023]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v9 02/10] rockchip-mailbox: Fix typo
    On Tue, Dec 27, 2022 at 11:42:36AM -0600, Allen Webb wrote:
    > On Tue, Dec 20, 2022 at 5:09 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
    > > I think it would make sense then to be explicit about this for now, even
    > > if it seems we can obsolete this. Right now the justification for having
    > > this for built-in is *very* specific to this feature for USB, which
    > > makes use of special USB sysfs attributes which as you explained, allows
    > > to restrict probe of devices even though the respective driver is already
    > > loaded.
    >
    > The thing we might obsolete is limiting it to just the USB subsystem.
    > I am fine with expanding the documentation and limiting the scope of
    > the feature to USB/thunderbolt for now.

    Great let's do that as otherwise it can leave a few folks scratchign
    their head.

    > > > There are sysfs attributes called authorized and authorized_default
    > > > that together can prevent devices from being fully enumerated and
    > > > probed.
    > >
    > > Although these attributes are USB specfic today it gets me wondering if
    > > other subsystems may benefit from a similar feature.
    >
    > The subsystems that would likely benefit the most are ones that are
    > externally reachable.

    Makes sense.

    > The external ports that come to mind are USB /
    > thunderbolt, firewire, PCMCIA / expresscard, eSATA, serial and
    > parallel ports. Supporting PCMCIA / expresscard seems like it would
    > require adding the authorized sysfs attribute to pci. eSATA would be
    > covered by ata.

    Makes sense, I'd personally ignore anything legacy such as PCMCIA though.

    > > > authorized_default gets set to 0 for the hub and any devices
    > > > connected after that will show in sysfs, but not fully enumerate or
    > > > probe until the device's authorized attribute is set to 1. There are
    > > > some edge cases like internal devices which have some extra
    > > > complexity.
    > > >
    > > > As for documentation, I wasn't able to find much other than:
    > > > https://github.com/torvalds/linux/blob/v6.1/drivers/usb/core/hcd.c#L370
    > > > /* authorized_default behaviour:
    > > > * -1 is authorized for all devices except wireless (old behaviour)
    > > > * 0 is unauthorized for all devices
    > > > * 1 is authorized for all devices
    > > > * 2 is authorized for internal devices
    > > > */
    > > > ...
    > > > and
    > > > https://github.com/torvalds/linux/blob/v6.1/Documentation/admin-guide/kernel-parameters.txt#L6424
    > > > usbcore.authorized_default=
    > > > [USB] Default USB device authorization:
    > > > (default -1 = authorized except for wireless USB,
    > > > 0 = not authorized, 1 = authorized, 2 = authorized
    > > > if device connected to internal port)
    > > > ...
    > > > The feature looks like it was originally introduced for wireless USB in:
    > > > https://www.mail-archive.com/linux-usb-devel@lists.sourceforge.net/msg54289.html
    > > > and later adapted for use cases like USBGuard here:
    > > > https://github.com/torvalds/linux/commit/c4fc2342cb611f945fa468e742759e25984005ad
    > >
    > > Thanks for digging all this up. Can you extend the docs on
    > > Documentation/driver-api/usb/ somewhere about this attribute as part of
    > > your changes so its clear the motivation, *then* you make your changes.
    > > The documentation for MODULE_DEVICE_TABLE() can just say:
    > >
    > > The only use-case for built-in drivers today is enable userspace to
    > > prevent / allow probe for devices on certain subsystems even if the
    > > driver is already loaded. An example is the USB subsystem with its
    > > authorized_default sysfs attribute. For more details refer to the
    > > kernel's Documentation for USB about authorized_default.
    > >
    > > That should be clear enough for both USB driver writers and others.
    > >
    > > Please also extend the docs for MODULE_DEVICE_TABLE() on
    > > Documentation/driver-api/usb/writing_usb_driver.rst or where you see
    > > fit for your changes. That can go into depth about the USBGuard stuff.
    > >
    > > Luis
    >
    > How do you feel about only having one version of the macro for both
    > cases and merging the documentation so things are kept simple? Here is
    > what I have locally for the macro without the ifdef and the updated
    > documentation:
    >
    > /*
    > * Creates an alias so file2alias.c can find device table.
    > *
    > * Use this in cases where a device table is used to match devices because it
    > * surfaces match-id based module aliases to userspace for:
    > * - Automatic module loading through modules.alias.
    > * - Tools like USBGuard which allow or block devices based on policy such as
    ^ allow to

    > * which modules match a device.
    > *
    > * The only use-case for built-in drivers today is enable userspace to prevent /

    ^ is to

    > * allow probe for devices on certain subsystems even if the driver is already
    > * loaded. An example is the USB subsystem with its authorized_default sysfs
    > * attribute. For more details refer to the kernel's Documentation for USB about
    > * authorized_default.
    > *
    > * The module name is included in the alias for two reasons:
    > * - It avoids creating two aliases with the same name for built-in modules.
    > * Historically MODULE_DEVICE_TABLE was a no-op for built-in modules, so
    > * there was nothing to stop different modules from having the same device
    > * table name and consequently the same alias when building as a module.
    > * - The module name is needed by files2alias.c to associate a particular
    > * device table with its associated module for built-in modules since
    > * files2alias would otherwise see the module name as `vmlinuz.o`.

    Yeah sure this reads much better.

    > */
    > #define MODULE_DEVICE_TABLE(type, name) \
    > extern void *CONCATENATE( \
    > CONCATENATE(__mod_##type##__##name##__, \
    > __KBUILD_MODNAME), \
    > _device_table) \
    > __attribute__ ((unused, alias(__stringify(name))))
    >
    >
    > Here is a draft version for an updated to
    > Documentation/driver-api/usb/ (I will add the 80 char line breaks
    > later) in case you have feedback:
    >
    >
    > # Authorization
    >
    > Authorization provides userspace a way to allow or block configuring
    > devices early during enumeration before any modules are probed for the
    > device. While it is possible to block a device by not loading the
    > required modules, this also prevents other devices from using the
    > module as well. For example someone might have an unattended computer
    > downloading installation media to a USB drive. Presumably this
    > computer would be locked to make it more difficult for a bad actor to
    > access the computer. Since USB storage devices are not needed to
    > interact with the lock screen, the authorized_default sysfs attribute
    > can be set to not authorize new USB devices by default. A userspace
    > tool like USBGuard can then vet the devices. Mice, keyboards, etc can
    > be allowed by writing to their authorized sysfs attribute so that the
    > lock screen can still be used (this important in cases like
    > suspend+resume or docks) while other devices can be blocked as long as
    > the lock screen is shown.
    >
    > ## Sysfs Attributes
    >
    > Userspace can control USB device authorization through the
    > authorized_default and authorized sysfs attributes.
    >
    > ### authorized_default
    >
    > .. kernel-doc:: drivers/usb/core/hcd.c
    > :export:
    >
    > The authorized_default sysfs attribute is only present for host
    > controllers. It determines the initial state of the authorized sysfs
    > attribute of USB devices newly connected to the corresponding host
    > controller. It can take on the following values:
    >
    > +---------------------------------------------------+
    > | Value | Behavior |
    > +=======+===========================================+
    > | -1 | Authorize all devices except wireless USB |
    > +-------+-------------------------------------------+
    > | 0 | Do not authorize new devices |
    > +-------+-------------------------------------------+
    > | 1 | Authorize new devices |
    > +-------+-------------------------------------------+
    > | 2 | Authorize new internal devices only |
    > +---------------------------------------------------+
    >
    > Note that firmware platform code determines if a device is internal or
    > not and this is reported as the connect_type sysfs attribute of the
    > USB port. This is currently supported by ACPI, but device tree still
    > needs an implementation. Authorizing new internal devices only can be
    > useful to work around issues with devices that misbehave if there are
    > delays in probing their module.
    >
    > ### authorized
    >
    > .. kernel-doc:: drivers/usb/core/sysfs.c
    > :export:
    >
    > Every USB device has an authorized sysfs attribute which can take the
    > values 0 and 1. When authorized is 0, the device still is present in
    > sysfs, but none of its interfaces can be associated with drivers and
    > modules will not be probed. When authorized is 1 (or set to one) a
    > configuration is chosen for the device and its interfaces are
    > registered allowing drivers to bind to them.

    Good stuff!

    Luis

    \
     
     \ /
      Last update: 2023-03-26 23:34    [W:4.069 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site