netdev - Re: [Xen-devel] xen-netfront possibly rides the rocket too often

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: <20140516102252.GE18551@zion.uk.xensource.com>
Date:	Fri, 16 May 2014 11:22:52 +0100
From:	Wei Liu <wei.liu2@...rix.com>
To:	David Laight <David.Laight@...LAB.COM>
CC:	'Wei Liu' <wei.liu2@...rix.com>,
	Stefan Bader <stefan.bader@...onical.com>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	Zoltan Kiss <zoltan.kiss@...rix.com>,
	"xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
	netdev <netdev@...r.kernel.org>
Subject: Re: [Xen-devel] xen-netfront possibly rides the rocket too often

On Fri, May 16, 2014 at 10:05:46AM +0000, David Laight wrote:
[...]
> > ---8<---
> > From 743495a2b2d338fc6cfe9bfd4b6e840392b87f4a Mon Sep 17 00:00:00 2001
> > From: Wei Liu <wei.liu2@...rix.com>
> > Date: Fri, 16 May 2014 10:39:01 +0100
> > Subject: [PATCH] xen-netfront: linearize SKB if it occupies too many slots
> > 
> > Some workload, such as Redis can generate SKBs which make use of compound
> > pages. Netfront doesn't quite like that because it doesn't want to send
> > exessive slots to the backend as backend might deem it malicious. On the
> > flip side these packets are actually legit, the size check at the
> > beginning of xennet_start_xmit ensures that packet size is below 64K.
> > 
> > So we linearize SKB if it occupies too many slots. If the linearization
> > fails then the SKB is dropped.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@...rix.com>
> > ---
> >  drivers/net/xen-netfront.c |   18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> > index 895355d..0361fc5 100644
> > --- a/drivers/net/xen-netfront.c
> > +++ b/drivers/net/xen-netfront.c
> > @@ -573,9 +573,21 @@ static int xennet_start_xmit(struct sk_buff *skb, struct net_device *dev)
> >  	slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
> >  		xennet_count_skb_frag_slots(skb);
> >  	if (unlikely(slots > MAX_SKB_FRAGS + 1)) {
> > -		net_alert_ratelimited(
> > -			"xennet: skb rides the rocket: %d slots\n", slots);
> > -		goto drop;
> > +		if (skb_linearize(skb)) {
> 
> You don't need to actually linearize the skb here.
> One with multiple fragments is fine.

Yes I know; but my idea is to get a SKB that doesn't use up too many
slots -- a linearized SKB will not use too many slots.

> I'm not sure there is a standard function to 'copy and refragment'
> the skb data though.
> 

That can only help if we can control how the fragment page is allocated.

> > +			net_alert_ratelimited(
> > +				"xennet: failed to linearize skb, skb dropped\n");
> > +			goto drop;
> > +		}
> > +		data = skb->data;
> > +		offset = offset_in_page(data);
> > +		len = skb_headlen(skb);
> > +		slots = DIV_ROUND_UP(offset + len, PAGE_SIZE) +
> > +			xennet_count_skb_frag_slots(skb);
> 
> IIRC If you have called skb_linearize then there shouldn't be any fragments.
> 

Plain copy-n-paste from code above. I will pay more attention if it's a
formal patch. ;-)

Wei.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html