On Mon, May 25, 2026 at 08:42:01AM -0400, Michael S. Tsirkin wrote:
On Mon, May 25, 2026 at 11:53:14AM +0100, David Laight wrote:
On Mon, 25 May 2026 11:57:45 +0200
Stefano Garzarella <[email protected]> wrote:
> On Sat, May 23, 2026 at 05:35:57PM +0100, David Laight wrote:
> >On Sat, 23 May 2026 02:20:29 +0000
> >[email protected] wrote:
> >
> >> Hello:
> >>
> >> This patch was applied to netdev/net.git (main)
> >> by Jakub Kicinski <[email protected]>:
> >
> >Did anyone else notice that is isn't a bug?
> >
> >There is no way that a 'count of bytes of kernel memory' can overflow
> >the size of 'long'.
>
> It's more of an estimate than an actual calculation of memory usage if
> we queue the incoming packet. In theory, an overflow could occur if the
> user sets `buf_alloc` to 4GB. In practice, though, I think you're right:
> the memory should run out before we get to that check.
The calculation is:
u64 skb_overhead = (skb_queue_len(&vvs->rx_queue) + 1) *
SKB_TRUESIZE(0);
skb_queue_len() will be the number of items on the queue.
SKB_TRUESIZE(0) is the memory taken up by a zero length skb (basically
sizeof(skb)).
Unless you either corrupt the queue length or manage to allocate skb that use
less than the minimum about of memory that product can't overflow 'unsigned
long'.
The later calculations might wrap - but the multiply can't.
-- David
Indeed, I wasn't thinking. For this to even get close to overflowing
we'd have to have almost all of 4G available to the 32 bit kernel taken
up by this single queue.
Revert, I'd say.
I also blindly added the cast to silence sashiko :-(
I see now that it could never actually happen, but semantically it’s
correct, so maybe we can avoid the revert.
Thanks,
Stefano
>
> Thanks,
> Stefano
>
> >
> >-- David
> >
> >>
> >> On Thu, 21 May 2026 14:47:32 +0200 you wrote:
> >> > From: Stefano Garzarella <[email protected]>
> >> >
> >> > On 32-bit architectures, both skb_queue_len() and SKB_TRUESIZE(0)
evaluate
> >> > to 32-bit values. The multiplication can overflow before being assigned
to
> >> > the u64 skb_overhead variable, making the skb overhead check ineffective.
> >> >
> >> > Cast skb_queue_len() to u64 so the multiplication is always performed in
> >> > 64-bit arithmetic.
> >> >
> >> > [...]
> >>
> >> Here is the summary with links:
> >> - [net] vsock/virtio: fix skb overhead overflow on 32-bit builds
> >> https://git.kernel.org/netdev/net/c/4157501b9a8f
> >>
> >> You are awesome, thank you!
> >
>
