Thread (10 messages) 10 messages, 6 authors, 2024-10-08

Re: [PATCH net] net: ethernet: adi: adin1110: Fix some error handling path in adin1110_read_fifo()

From: Simon Horman <horms@kernel.org>
Date: 2024-10-04 11:37:39
Also in: kernel-janitors, lkml

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".
At least that is what my brief investigation tells me.
Return -EINVAL instead.
Please include some information on how this was found and tested.
e.g.

Found by inspection / Found using widget-ng.
Compile tested only.
Fixes: bc93e19d088b ("net: ethernet: adi: Add ADIN1110 support")
Signed-off-by: Christophe JAILLET <redacted>
---
This patch is speculative.
If returning 0 is what was intended, then an explicit 0 would be better.
In my brief investigation I see that adin1110_read_fifo()
is only called by adin1110_read_frames(), like this:

	while (budget) {
		...

		ret = adin1110_read_fifo(port_priv);
		if (ret < 0)
			return;

		budget--;
	}

So the question becomes, should a failure in reading the fifo,
because of an invalid frame size, be treated as an error
and terminate reading frames.

Like you, I speculate the answer is yes.
But I think we need a bit more certainty to take this patch.

-- 
pw-bot: under-review
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help