linux-kernel - Re: [PATCH] usb: typec: Add bus type for plug alt modes

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: <20201209161356.GI680328@kuha.fi.intel.com>
Date:   Wed, 9 Dec 2020 18:13:56 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Prashant Malani <pmalani@...omium.org>
Cc:     "open list:USB NETWORKING DRIVERS" <linux-usb@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Benson Leung <bleung@...omium.org>,
        Jonathan Corbet <corbet@....net>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: typec: Add bus type for plug alt modes

On Tue, Dec 08, 2020 at 03:45:19PM -0800, Prashant Malani wrote:
> Hi Heikki,
> 
> Thanks a lot for looking at the patch.
> 
> On Tue, Dec 8, 2020 at 1:37 AM Heikki Krogerus <heikki.krogerus@...ux.intel.com> wrote:
> >
> > On Wed, Dec 02, 2020 at 07:08:47PM -0800, Prashant Malani wrote:
> > > Add the Type C bus for plug alternate modes which are being
> > > registered via the Type C connector class. This ensures that udev events
> > > get generated when plug alternate modes are registered (and not just for
> > > partner/port alternate modes), even though the Type C bus doesn't link
> > > plug alternate mode devices to alternate mode drivers.
> >
> > I still don't understand how is the uevent related to the bus? If you
> > check the device_add() function, on line 2917, kobject_uevent() is
> > called unconditionally. The device does not need a bus for that event
> > to be generated.
> 
> My initial thought process was to see what is the difference in the adev device
> initialization between partner altmode and plug altmode (the only difference I saw in
> typec_register_altmode() was regarding the bus field).
> 
> Yes, kobject_uevent() is called unconditionally, but it's return value isn't checked,
> so we don't know if it succeeded or not.
> 
> In the case of cable plug altmode, I see it fail with the following error[1]:
> 
> [  114.431409] kobject: 'port1-plug0.0' (000000004ad42956): kobject_uevent_env: filter function caused the event to drop!
> 
> I think the filter function which is called is this one: drivers/base/core.c: dev_uevent_filter() [2]
> 
> static int dev_uevent_filter(struct kset *kset, struct kobject *kobj)
> {
> 	struct kobj_type *ktype = get_ktype(kobj);
> 
> 	if (ktype == &device_ktype) {
> 		struct device *dev = kobj_to_dev(kobj);
> 		if (dev->bus)
> 			return 1;
> 		if (dev->class)
> 			return 1;
> 	}
> 	return 0;
> }
> 
> So, both the "if (dev->bus)" and "if (dev->class)" checks are failing here. In the case of partner alt modes, bus is set by the class.c code
> so this check likely returns 1 in that case.

OK. I understand the issue now. So I would say that the proper
solution to this problem is to link the alt modes with the class
instead of the bus. That is much smaller change IMO.


thanks,

-- 
heikki