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

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: <20250113125232.733fb088@kernel.org>
Date: Mon, 13 Jan 2025 12:52:32 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Foster Snowhill <forst@....gy>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Georgi Valkov
 <gvalkov@...il.com>, Simon Horman <horms@...nel.org>, Oliver Neukum
 <oneukum@...e.com>, netdev@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH net v4 0/7] usbnet: ipheth: prevent OoB reads of NDP16

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.