Re: [PATCH net v4 0/7] usbnet: ipheth: prevent OoB reads of NDP16

[Jakub Kicinski <kuba@kernel.org>]

On Mon, 13 Jan 2025 02:48:58 +0100 Foster Snowhill wrote:
> Thank you very much for the review!
> 
> I went through the series again, noticed a couple minor things I think
> I should fix:
> 
> * Patch 1/7 ("usbnet: ipheth: break up NCM header size computation")
>   [p1] introduces two new preprocessor constants. Only one of them is
>   used (the other one is intermediate, for clarity), and the usage is
>   all the way in patch 6/7 ("usbnet: ipheth: fix DPE OoB read") [p6].
>   I'd like to move the constant introduction patch right before the
>   patch that uses one of them. There's no good reason they're spread
>   out like they are in v4.
> * Commit message in patch 5/7 ("usbnet: ipheth: refactor NCM datagram
>   loop") [p5] has a stray paragraph starting with "Fix an out-of-bounds
>   DPE read...". This needs to be removed.
> 
> I'd like to get this right. I'll make the changes above, add Cc stable,
> re-test all patches in sequence, and submit v5 soon. As this will be
> a different revision, I figure I can't formally apply your "Reviewed-by"
> anymore, the series may need another look once I post v5.

The opinions on the exact rules differ but you can definitely add my tag
on the patches which won't change.

> Also I have some doubts about patch 7/7 [p7] with regards to its
> applicability to backporting to older stable releases. This only adds a
> documentation comment, without fixing any particular issue. Doesn't
> sound like something that should go into stable. But maybe fine if it's
> part of a series?

Yes, it's fine as part of the series.

> I can also add that text in a commit message rather
> than the source code of the driver itself, or even just keep it in the
> cover letter. Do you have any opinion on this?

Maybe it's because I don't work with USB networking much but to me
the comment was useful.