linux-kernel - Re: [PATCH v1] driver: base: Add driver filter support

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: <CAPcyv4iCBknhGyw-YjO7_Tua9Vkw_UCSHVj3prL3mVfz4nj-_g@mail.gmail.com>
Date:   Thu, 5 Aug 2021 09:37:28 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Andi Kleen <ak@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Kuppuswamy Sathyanarayanan <knsathya@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v1] driver: base: Add driver filter support

On Thu, Aug 5, 2021 at 12:58 AM Greg Kroah-Hartman
<gregkh@...uxfoundation.org> wrote:
>
> On Thu, Aug 05, 2021 at 09:55:33AM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Aug 05, 2021 at 09:49:29AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Aug 04, 2021 at 12:50:24PM -0700, Andi Kleen wrote:
> > > > > And what's wrong with the current method of removing drivers from
> > > > > devices that you do not want them to be bound to?  We offer that support
> > > > > for all busses now that want to do it, what driver types are you needing
> > > > > to "control" here that does not take advantage of the existing
> > > > > infrastructure that we currently have for this type of thing?
> > > >
> > > > I'm not sure what mechanism you're referring to here, but in general don't
> > > > want the drivers to initialize at all because they might get exploited in
> > > > any code that they execute.
> > >
> > > That is exactly the mechanism we have today in the kernel for all busses
> > > if they wish to take advantage of it.  We have had this for all USB
> > > drivers for well over a decade now, this is not a new feature.  Please
> > > use that instead.
> >
> > Hm, wait, maybe that didn't get merged yet, let me dig...
> >
>
> Ok, my fault, I was thinking of the generic "removable" support that
> recently got added.
>
> Both thunderbolt and USB have the idea of "authorized" devices, that is
> the logic that should be made generic and available for all busses to
> use, by moving it to the driver core, just like the "removable" logic
> got moved to the driver core recently (see 70f400d4d957 ("driver core:
> Move the "removable" attribute from USB to core")
>
> Please use that type of interface, as we already have userspace tools
> using it, and expand it for all busses in the system to use if they
> want.  Otherwise with this proposal you will end up with multiple ways
> to control the same bus type with different types of "filtering",
> ensuring a mess.

I overlooked the "authorized" attribute in usb and thunderbolt. The
collision problem makes sense. Are you open to a core "authorized"
attribute that buses like usb and thunderbolt would override in favor
of their local implementation? I.e. similar to suppress_bind_attrs:

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index daeb9b5763ae..d1780f026d1a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -511,6 +511,10 @@ static int call_driver_probe(struct device *dev,
struct device_driver *drv)
 {
        int ret = 0;

+       if (driver_core_auth_enabled && !dev->bus->suppress_authorized_attrs &&
+           !driver_core_authorized(dev))
+               return -ENODEV;
+
        if (dev->bus->probe)
                ret = dev->bus->probe(dev);
        else if (drv->probe)
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index a062befcb3b2..e835be9bee4f 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -323,6 +323,7 @@ struct bus_type tb_bus_type = {
        .probe = tb_service_probe,
        .remove = tb_service_remove,
        .shutdown = tb_service_shutdown,
+       .suppress_authorized_attrs = true,
 };

 static void tb_domain_release(struct device *dev)
diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index 072968c40ade..2cf9c3cc12b4 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -2028,4 +2028,5 @@ struct bus_type usb_bus_type = {
        .match =        usb_device_match,
        .uevent =       usb_uevent,
        .need_parent_lock =     true,
+       .suppress_authorized_attrs = true,
 };