netdev - Re: mt76: dealing with disconnections, -ENODEV and -EPROTO

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: <20241220002759.34dcfe11@foxbook>
Date: Fri, 20 Dec 2024 00:27:59 +0100
From: Michał Pecio <michal.pecio@...il.com>
To: aleksander.lobakin@...el.com
Cc: Bo.Jiao@...iatek.com, StanleyYP.Wang@...iatek.com,
 alexander.deucher@....com, andrew+netdev@...n.ch, andrew@...n.ch,
 angelogioacchino.delregno@...labora.com, apais@...ux.microsoft.com,
 chui-hao.chiu@...iatek.com, cug_yangyuancong@...mail.com,
 daniel@...rotopia.org, davem@...emloft.net, deren.wu@...iatek.com,
 dokyungs@...sei.ac.kr, dsahern@...nel.org, dzm91@...t.edu.cn,
 edumazet@...gle.com, gch981213@...il.com, git@...nap.io,
 gregkh@...uxfoundation.org, guanwentao@...ontech.com,
 gustavoars@...nel.org, helmut.schaa@...glemail.com, horms@...nel.org,
 jiefeng_li@...t.edu.cn, keescook@...omium.org, kuba@...nel.org,
 kvalo@...eaurora.org, kvalo@...nel.org, leit@...com, leitao@...ian.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-mediatek@...ts.infradead.org, linux-usb@...r.kernel.org,
 linux-wireless@...r.kernel.org, lorenzo.bianconi83@...il.com,
 lorenzo.bianconi@...hat.com, lorenzo@...nel.org, lynxis@...0.eu,
 mailhol.vincent@...adoo.fr, markus.theil@...ilmenau.de,
 matthias.bgg@...il.com, mikhail.v.gavrilov@...il.com,
 mingyen.hsieh@...iatek.com, mrkiko.rs@...il.com, nbd@....name,
 nelson.yu@...iatek.com, netdev@...r.kernel.org, oliver@...kum.org,
 pabeni@...hat.com, quan.zhou@...iatek.com, raoxu@...ontech.com,
 rodrigo.vivi@...el.com, romain.perier@...il.com, rong.yan@...iatek.com,
 ryder.lee@...iatek.com, sean.wang@...iatek.com, sgruszka@...hat.com,
 shayne.chen@...iatek.com, sidhayn@...il.com, stern@...land.harvard.edu,
 stf_xl@...pl, sujuan.chen@...iatek.com, wang.zhao@...iatek.com,
 wangyuli@...ontech.com, weiwan@...gle.com, woojung.huh@...rochip.com,
 yn.chen@...iatek.com, zhanjun@...ontech.com
Subject: Re: mt76: dealing with disconnections, -ENODEV and -EPROTO

> >  		ret = usb_control_msg(udev, pipe, req, req_type, val,
> >  				      offset, buf, len, MT_VEND_REQ_TOUT_MS);
> > -		if (ret == -ENODEV)
> > +		if (ret == -ENODEV || ret == -EPROTO)
> >  			set_bit(MT76_REMOVED, &dev->phy.state);
> > -		if (ret >= 0 || ret == -ENODEV)
> > +		if (ret >= 0 || ret == -ENODEV || ret == -EPROTO)
> >  			return ret;
> >  		usleep_range(5000, 10000);
> 
> How do other drivers handle this?
> Can -EPROTO happen in other cases, not only unplugging, which this
> patch would break?

Yes, -EPROTO may be a transient error, although they are relatively
rare as some retries are done before the URB fails. This patch will
only break things if things work in the first place, i.e. the driver
has sensible retry policies, the hardware doesn't get confused, etc.

Note that -EPROTO is not guaranteed in this case, see
Documentation/driver-api/usb/error-codes.rst

I know that xHCI gives -EPROTO and it looks like EHCI does too (IDK
if this is reliable), but I just checked that OHCI gives -ETIME.


I don't have this hardware, but I played with some other wired/WiFi
dongles and observed similar problems of varying severity.

It looks to me like USB core doesn't actually return -ENODEV on
disconnected devices, or at least doesn't react to disconnection
until .probe() returns, I am not yet sure which of those.

And drivers don't seem to expect -EPROTO or -ETIME to be fatal.

And maybe they should, if they wouldn't be able to recover from it
anyway, and I know that there are drivers which can't. But I think
this USB subsystem behavior is suboptimal too.

Regards,
Michal