linux-kernel - Re: [PATCH v1] usb: typec: tcpm: only discover modes the port supports svids for

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: <ZT9bBnKPMTDqy6aW@kuha.fi.intel.com>
Date:   Mon, 30 Oct 2023 09:30:34 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     RD Babiera <rdbabiera@...gle.com>
Cc:     gregkh@...uxfoundation.org, linux@...ck-us.net,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        badhri@...gle.com, stable@...r.kernel.org
Subject: Re: [PATCH v1] usb: typec: tcpm: only discover modes the port
 supports svids for

Hi,

I'm sorry to keep you waiting.

<snip>

> > If you need the modes to be discovered in some specific order, then we
> > need the framework to allow you to do that. So perhaps the tcpci
> > drivers should be able to supply the preferred order to the tcpm?
> >
> > But as such, unless I'm mistaken, this patch will change the logic so
> > that only the partner alt modes that the port supports get registered,
> > and that way are exposed to the user. You can't do that - right now
> > it's the only way we can inform the user about them. All partner
> > alternate modes (at least the SVIDs) must be exposed to the user one
> > way or the other, regardless does the port support them or not.
> 
> The test this patch tries to fix could just be written without consideration
> of this. My guess is that the test was designed such that the SVIDs before
> the DisplayPort SVID are unknown to the port under test so the mentality
> could have been "why should a port care about SVIDs it doesn't know
> about?"
> 
> A defense I could make for it is that the USB PD CTS doesn't test
> to see if a port under test sends Discover Modes for every SVID returned
> in a Discover SVIDs ACK, so the interpretation isn't invalid. I've seen other
> tcpm implementations handle Discover Modes this way as well.
> 
> Regardless, you're definitely right that the user should know about all
> Alt Modes/SVIDs - the port would lose SVID information without
> registering a partner altmode for it. Currently I think the approaches to pass
> this test look like:
>     1. Your suggestion - let the tcpci decide if there should be a
> discovery order.
> Alternatively, let the tcpci decide if it wants to opt into this
> patch's behavior of
> only discovering modes for known SVIDs - a strict discovery flag.
>     2. Send a Discover Mode message to known SVIDs first in the order
> they come in, and then to unknown SVIDs. The test passes and no information
> is lost, but it's unnecessary refactoring just to pass one test for
> one Alt Mode.
>     3. Don't send a Discover Mode message to unknown SVIDs, but do register
> an Alt Mode with blank info for that SVID. It passes the test without having to
> do any reordering compared to the first option and it preserves supported
> SVIDs. But, the port would lose information such as each SVID's Alt Modes
> VDO plus each SVID can support more than one Alt Mode.
> 
> Let me know if any of these approaches sound worth pursuing; I do think
> Option 1 does make more sense than the others.

I would like to hear what Guenter thinks. Guenter, do you have time to
take a look at this?

thanks,

-- 
heikki