Re: [PATCH v5 1/2] usb/typec: fix array overruns in ucsi.c partner_altmode[]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Sep 10, 2020 at 12:52:05AM -0700, Zwane Mwaikambo wrote:
>  Hi Heikki,
> 
> On Wed, 9 Sep 2020, Heikki Krogerus wrote:
> 
> > On Thu, Sep 03, 2020 at 02:10:50PM +0300, Heikki Krogerus wrote:
> > > Hi Zwane,
> > > 
> > > Sorry to keep you waiting. I'm trying to find a board I can use to
> > > test these...
> > 
> > I've now tested this (these) with ThinkPad X280, and there is no
> > regression, however, now that (I think) I understand what's going on,
> > I would not try to fix the issue like you do. I would simply make sure
> > the alternate mode arrays are NULL terminated. For example with
> > something like this:
> > 
> > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> > index cba6f77bea61b..7e66e4d232996 100644
> > --- a/drivers/usb/typec/ucsi/ucsi.h
> > +++ b/drivers/usb/typec/ucsi/ucsi.h
> > @@ -317,8 +317,8 @@ struct ucsi_connector {
> >         struct typec_port *port;
> >         struct typec_partner *partner;
> >  
> > -       struct typec_altmode *port_altmode[UCSI_MAX_ALTMODES];
> > -       struct typec_altmode *partner_altmode[UCSI_MAX_ALTMODES];
> > +       struct typec_altmode *port_altmode[UCSI_MAX_ALTMODES + 1];
> > +       struct typec_altmode *partner_altmode[UCSI_MAX_ALTMODES + 1];
> >  
> >         struct typec_capability typec_cap;
> > 
> > Though I'm not sure we should need even that. Try it out in any case.
> > 
> > Even if that works, I have a feeling there is something odd going on.
> > What kinds of device has all 30 modes supported (or even more)? I want
> > to know if this is a case where the firmware is just reporting bogus
> > values.
> > 
> > What device are you plugging to the Type-C connector? Does it really
> > have all 30 altmodes? What do you see in /sys/class/typec directory
> > when you connect the device?
> > 
> >         ls /sys/class/typec
> > 
> > Actually, do this:
> > 
> >         grep . /sys/class/typec/port*-partner/port*-partner.*/svid
> > 
> > and tell what you get.
> 
> Thanks for digging into it, the device being plugged in is a Lenovo TB 
> dock 
> (https://www.lenovo.com/ca/en/accessories-and-monitors/home-office/Thunderbolt-Dock-Gen-2-US/p/40AN0135US);

It has TBT, DP, and probable the Lenovo vendor specific mode. So two
or three modes, no more, so not 30.

> thinkpad :: ~ » ls /sys/class/typec
> port0
> 
> thinkpad :: /sys/class/typec » grep . /sys/class/typec/port*-partner/port*-partner.*/svid
> zsh: no matches found: /sys/class/typec/port*-partner/port*-partner.*/svid

How can you have partner change notifications without a partner? I'm
probable still missing something. I wonder what exactly do you have in
the partner alternate mode array? I think your patche(s) are working
around some deeper issue. We really need to figure out what that is.

Let's check the trace output. You need to build the UCSI drivers as
modules. Then:

        modprobe -r ucsi_acpi
        modprobe typec_ucsi
        mount debugfs -t debugfs /sys/kernel/debug
        cd /sys/kernel/debug/tracing
        echo 1 > events/ucsi/enable
        modprobe ucsi_acpi

Wait one minute and:

        cat trace

thanks,

-- 
heikki



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux