netdev - Re: [PATCH net] net: ethernet: adi: adin1110: Fix some error handling path in adin1110_read

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: <20241007154505.GG32733@kernel.org>
Date: Mon, 7 Oct 2024 16:45:05 +0100
From: Simon Horman <horms@...nel.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Lennart Franzen <lennart@...omain.com>,
	Alexandru Tachici <alexandru.tachici@...log.com>,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH net] net: ethernet: adi: adin1110: Fix some error
 handling path in adin1110_read_fifo()

On Fri, Oct 04, 2024 at 03:15:39PM +0200, Christophe JAILLET wrote:
> Le 04/10/2024 à 13:37, Simon Horman a écrit :
> > On Thu, Oct 03, 2024 at 08:53:15PM +0200, Christophe JAILLET wrote:
> > > If 'frame_size' is too small or if 'round_len' is an error code, it is
> > > likely that an error code should be returned to the caller.
> > > 
> > > Actually, 'ret' is likely to be 0, so if one of these sanity checks fails,
> > > 'success' is returned.
> > 
> > Hi Christophe,
> > 
> > I think we can say "'ret' will be 0".
> 
> Agreed.
> 
> 	ret = adin1110_read_reg()
> 	--> spi_sync_transfer()
> 	--> spi_sync()
> 
> which explicitly documents "zero on success, else a negative error code."
> 
> > At least that is what my brief investigation tells me.
> > 
> > > 
> > > Return -EINVAL instead.
> > 
> 
> If the patch is considered as correct, can you confirm that -EINVAL is the
> correct error code to use? If not, which one would be preferred?

-EINVAL seems reasonable to me.

> > Please include some information on how this was found and tested.
> > e.g.
> > 
> > Found by inspection / Found using widget-ng.
> 
> I would say: found by luck! :)
> 
> The explanation below will be of no help in the commit message and won't be
> added. I just give you all the gory details because you asked for it ;-)
> 
> (and after reading bellow, you can call me crazy!)

:)