netdev - Re: [PATCH net] net: libwx: fix firmware mailbox abnormal return

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250102163749.66d047c9@kernel.org>
Date: Thu, 2 Jan 2025 16:37:49 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Jiawen Wu <jiawenwu@...stnetic.com>
Cc: <andrew+netdev@...n.ch>, <davem@...emloft.net>, <edumazet@...gle.com>,
 <pabeni@...hat.com>, <horms@...nel.org>, <rmk+kernel@...linux.org.uk>,
 <netdev@...r.kernel.org>, <mengyuanlou@...-swift.com>
Subject: Re: [PATCH net] net: libwx: fix firmware mailbox abnormal return

On Thu, 2 Jan 2025 14:42:30 +0800 Jiawen Wu wrote:
> > >  	/* Check command completion */
> > >  	if (status) {
> > >  		wx_dbg(wx, "Command has failed with no status valid.\n");
> > > -
> > > -		buf[0] = rd32(wx, WX_MNG_MBOX);
> > > -		if ((buffer[0] & 0xff) != (~buf[0] >> 24)) {
> > > -			status = -EINVAL;
> > > -			goto rel_out;
> > > -		}
> > > -		if ((buf[0] & 0xff0000) >> 16 == 0x80) {
> > > -			wx_dbg(wx, "It's unknown cmd.\n");
> > > -			status = -EINVAL;
> > > -			goto rel_out;
> > > -		}
> > >
> > > +		wx_dbg(wx, "check: %x %x\n", buffer[0] & 0xff, ~buf[0] >> 24);
> > > +		if ((buffer[0] & 0xff) != (~buf[0] >> 24))
> > > +			goto rel_out;  
> > 
> > Inverse question here, I guess. Why is it only an error for FW not
> > to be ready if cmd doesn't match?  
> 
> It is to check if the cmd has been processed by FW. If it matches, it means
> that FW has already processed the cmd, but FWRDY timeout for some
> unknown reason.

Feels a little risky. AFAICT the value we're comparing is just 
a checksum (potentially always set to 0xff?) It doesn't seem to
identify the command very well. If the command timed out let's
always return an error.

The other explanations make sense, please include them in the commit
msg.