netdev - RE: [PATCH v3 5/5] r8152: Block future register access if register access fails

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: <052401da00fa$dacccd90$906668b0$@realtek.com>
Date: Tue, 17 Oct 2023 13:07:17 +0000
From: Hayes Wang <hayeswang@...ltek.com>
To: "'Doug Anderson'" <dianders@...omium.org>
CC: "'Jakub Kicinski'" <kuba@...nel.org>,
        "'David S . Miller'"
	<davem@...emloft.net>,
        "'Alan Stern'" <stern@...land.harvard.edu>,
        "'Simon
 Horman'" <horms@...nel.org>,
        "'Edward Hill'" <ecgh@...omium.org>,
        "'Laura Nao'"
	<laura.nao@...labora.com>,
        "linux-usb@...r.kernel.org"
	<linux-usb@...r.kernel.org>,
        "'Grant Grundler'" <grundler@...omium.org>,
        'Bjørn Mork' <bjorn@...k.no>,
        "'Eric Dumazet'"
	<edumazet@...gle.com>,
        "'Paolo Abeni'" <pabeni@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH v3 5/5] r8152: Block future register access if register access fails

Doug Anderson <dianders@...omium.org>
> Sent: Tuesday, October 17, 2023 12:47 AM
[...
> > >  static int generic_ocp_read(struct r8152 *tp, u16 index, u16 size,
> > > @@ -8265,6 +8353,19 @@ static int rtl8152_pre_reset(struct
> usb_interface
> > > *intf)
> > >         if (!tp)
> > >                 return 0;
> > >
> > > +       /* We can only use the optimized reset if we made it to the end of
> > > +        * probe without any register access fails, which sets
> > > +        * `PROBED_WITH_NO_ERRORS` to true. If we didn't have that then return
> > > +        * an error here which tells the USB framework to fully unbind/rebind
> > > +        * our driver.
> >
> > Would you stay in a loop of unbind and rebind,
> > if the control transfers in the probe() are not always successful?
> > I just think about the worst case that at least one control always fails in probe().
> 
> We won't! :-) One of the first things that rtl8152_probe() does is to
> call rtl8152_get_version(). That goes through to
> rtl8152_get_version(). That function _doesn't_ queue up a reset if
> there are communication problems, but it does do 3 retries of the
> read. So if all 3 reads fail then we will permanently fail probe,
> which I think is the correct thing to do.

The probe() contains control transfers in
	1. rtl8152_get_version()
	2. tp->rtl_ops.init()

If one of the 3 control transfers in 1) is successful AND
any control transfer in 2) fails,
you would queue a usb reset which would unbind/rebind the driver.
Then, the loop starts.
The loop would be broken, if and only if
	a) all control transfers in 1) fail, OR
	b) all control transfers in 2) succeed.

That is, the loop would be broken when the fail rate of the control transfer is high or low enough.
Otherwise, you would queue a usb reset again and again.
For example, if the fail rate of the control transfer is 10% ~ 60%,
I think you have high probability to keep the loop continually.
Would it never happen?

Best Regards,
Hayes