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: <20250515104028.GQ3339421@horms.kernel.org>
Date: Thu, 15 May 2025 11:40:28 +0100
From: Simon Horman <horms@...nel.org>
To: Michael Kelley <mhklinux@...look.com>
Cc: kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
	decui@...rosoft.com, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	James.Bottomley@...senpartnership.com, martin.petersen@...cle.com,
	linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, linux-scsi@...r.kernel.org,
	stable@...r.kernel.org
Subject: Re: [PATCH net 3/5] hv_netvsc: Preserve contiguous PFN grouping in
 the page buffer array

On Wed, May 14, 2025 at 03:42:19PM +0000, Michael Kelley wrote:
> From: Simon Horman <horms@...nel.org> Sent: Wednesday, May 14, 2025 2:35 AM
> > 
> > On Mon, May 12, 2025 at 05:06:02PM -0700, mhkelley58@...il.com wrote:
> > > From: Michael Kelley <mhklinux@...look.com>

...

> > >  	for (i = 0; i < frags; i++) {
> > >  		skb_frag_t *frag = skb_shinfo(skb)->frags + i;
> > > +		struct hv_page_buffer *cur_pb = &pb[i + 2];
> > 
> > Hi Michael,
> > 
> > If I got things right then then pb is allocated on the stack
> > in netvsc_xmit and has MAX_DATA_RANGES elements.
> 
> Correct.
> 
> > 
> > If MAX_SKB_FRAGS is largs and MAX_DATA_RANGES has been limited to
> > MAX_DATA_RANGES. And frags is large. Is is possible to overrun pb here?
> 
> I don't think it's possible. Near the top of netvsc_xmit() there's a call
> to netvsc_get_slots(), along with code ensuring that all the data in the skb
> (and its frags) exists on no more than MAX_PAGE_BUFFER_COUNT (i.e., 32)
> pages. There can't be more frags than pages, so it should not be possible to
> overrun the pb array even if the frag count is large.
> 
> If the kernel is built with CONFIG_MAX_SKB_FRAGS greater than 30, and
> there are more than 30 frags in the skb (allowing for 2 pages for the rndis
> header), netvsc_xmit() tries to linearize the skb to reduce the frag count.
> But if that doesn't work, netvsc_xmit() drops the xmit request, which isn't
> a great outcome. But that's a limitation of the existing code, and this patch
> set doesn't change that limitation.

Hi Michael,

Thanks for addressing my concern. I do now see that the check
in netvsc_xmit() prevents an overrun.

And I agree that while not ideal dropping oversize packets is not
strictly related to this patch-set (and is indeed much better than
an overrun).

With the above in mind I'm now happy with this patch.

Reviewed-by: Simon Horman <horms@...nel.org>

...

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ