* [PATCH 3.2-3.12] skbuff: skb_segment: orphan frags before copying
[not found] ` <1397392513.10849.75.camel@deadeye.wl.decadent.org.uk>
@ 2014-04-13 22:57 ` Ben Hutchings
2014-04-13 23:20 ` David Miller
2014-04-13 23:02 ` [PATCH 3.2] ipv6: don't set DST_NOCOUNT for remotely added routes Ben Hutchings
1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2014-04-13 22:57 UTC (permalink / raw)
To: David Miller; +Cc: stable, Michael S. Tsirkin, Herbert Xu, netdev
[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]
From: "Michael S. Tsirkin" <mst@redhat.com>
commit 1fd819ecb90cc9b822cd84d3056ddba315d3340f upstream.
skb_segment copies frags around, so we need
to copy them carefully to avoid accessing
user memory after reporting completion to userspace
through a callback.
skb_segment doesn't normally happen on datapath:
TSO needs to be disabled - so disabling zero copy
in this case does not look like a big deal.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2. As skb_segment() only supports page-frags *or* a
frag list, there is no need for the additional frag_skb pointer or the
preparatory renaming.]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
For branches older than 3.6, commit a353e0ce0fd4 ('skbuff: add an api to
orphan frags') is needed before this. This is untested and I would
appreciate a review.
Ben.
---
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2699,6 +2699,9 @@ struct sk_buff *skb_segment(struct sk_bu
skb_put(nskb, hsize), hsize);
while (pos < offset + len && i < nfrags) {
+ if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
+ goto err;
+
*frag = skb_shinfo(skb)->frags[i];
__skb_frag_ref(frag);
size = skb_frag_size(frag);
--
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3.2-3.12] skbuff: skb_segment: orphan frags before copying
2014-04-13 22:57 ` [PATCH 3.2-3.12] skbuff: skb_segment: orphan frags before copying Ben Hutchings
@ 2014-04-13 23:20 ` David Miller
2014-04-13 23:50 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2014-04-13 23:20 UTC (permalink / raw)
To: ben; +Cc: stable, mst, herbert, netdev
From: Ben Hutchings <ben@decadent.org.uk>
Date: Sun, 13 Apr 2014 23:57:40 +0100
> From: "Michael S. Tsirkin" <mst@redhat.com>
>
> commit 1fd819ecb90cc9b822cd84d3056ddba315d3340f upstream.
>
> skb_segment copies frags around, so we need
> to copy them carefully to avoid accessing
> user memory after reporting completion to userspace
> through a callback.
>
> skb_segment doesn't normally happen on datapath:
> TSO needs to be disabled - so disabling zero copy
> in this case does not look like a big deal.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> [bwh: Backported to 3.2. As skb_segment() only supports page-frags *or* a
> frag list, there is no need for the additional frag_skb pointer or the
> preparatory renaming.]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> For branches older than 3.6, commit a353e0ce0fd4 ('skbuff: add an api to
> orphan frags') is needed before this. This is untested and I would
> appreciate a review.
I didn't do this backport because it seemed risky unless Michael
or someone else tested it thoroughly.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3.2-3.12] skbuff: skb_segment: orphan frags before copying
2014-04-13 23:20 ` David Miller
@ 2014-04-13 23:50 ` Ben Hutchings
0 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2014-04-13 23:50 UTC (permalink / raw)
To: David Miller; +Cc: stable, mst, herbert, netdev
[-- Attachment #1: Type: text/plain, Size: 1483 bytes --]
On Sun, 2014-04-13 at 19:20 -0400, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Sun, 13 Apr 2014 23:57:40 +0100
>
> > From: "Michael S. Tsirkin" <mst@redhat.com>
> >
> > commit 1fd819ecb90cc9b822cd84d3056ddba315d3340f upstream.
> >
> > skb_segment copies frags around, so we need
> > to copy them carefully to avoid accessing
> > user memory after reporting completion to userspace
> > through a callback.
> >
> > skb_segment doesn't normally happen on datapath:
> > TSO needs to be disabled - so disabling zero copy
> > in this case does not look like a big deal.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > [bwh: Backported to 3.2. As skb_segment() only supports page-frags *or* a
> > frag list, there is no need for the additional frag_skb pointer or the
> > preparatory renaming.]
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > For branches older than 3.6, commit a353e0ce0fd4 ('skbuff: add an api to
> > orphan frags') is needed before this. This is untested and I would
> > appreciate a review.
>
> I didn't do this backport because it seemed risky unless Michael
> or someone else tested it thoroughly.
Understood; I'll wait for further feedback.
Ben.
--
Ben Hutchings
I say we take off; nuke the site from orbit. It's the only way to be sure.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3.2] ipv6: don't set DST_NOCOUNT for remotely added routes
[not found] ` <1397392513.10849.75.camel@deadeye.wl.decadent.org.uk>
2014-04-13 22:57 ` [PATCH 3.2-3.12] skbuff: skb_segment: orphan frags before copying Ben Hutchings
@ 2014-04-13 23:02 ` Ben Hutchings
2014-04-13 23:19 ` David Miller
1 sibling, 1 reply; 6+ messages in thread
From: Ben Hutchings @ 2014-04-13 23:02 UTC (permalink / raw)
To: David Miller; +Cc: stable, Sabrina Dubroca, Hannes Frederic Sowa, netdev
[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]
From: Sabrina Dubroca <sd@queasysnail.net>
commit c88507fbad8055297c1d1e21e599f46960cbee39 upstream.
DST_NOCOUNT should only be used if an authorized user adds routes
locally. In case of routes which are added on behalf of router
advertisments this flag must not get used as it allows an unlimited
number of routes getting added remotely.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
[bwh: Backported to 3.2: adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
This is untested, but looks almost identical to what you sent for 3.4.
Please ack/nak.
Ben.
net/ipv6/route.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1250,7 +1250,7 @@ int ip6_route_add(struct fib6_config *cf
goto out;
}
- rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops, NULL, DST_NOCOUNT);
+ rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops, NULL, (cfg->fc_flags & RTF_ADDRCONF) ? 0 : DST_NOCOUNT);
if (rt == NULL) {
err = -ENOMEM;
--
Ben Hutchings
I haven't lost my mind; it's backed up on tape somewhere.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3.2] ipv6: don't set DST_NOCOUNT for remotely added routes
2014-04-13 23:02 ` [PATCH 3.2] ipv6: don't set DST_NOCOUNT for remotely added routes Ben Hutchings
@ 2014-04-13 23:19 ` David Miller
2014-04-13 23:49 ` Ben Hutchings
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2014-04-13 23:19 UTC (permalink / raw)
To: ben; +Cc: stable, sd, hannes, netdev
From: Ben Hutchings <ben@decadent.org.uk>
Date: Mon, 14 Apr 2014 00:02:22 +0100
> From: Sabrina Dubroca <sd@queasysnail.net>
>
> commit c88507fbad8055297c1d1e21e599f46960cbee39 upstream.
>
> DST_NOCOUNT should only be used if an authorized user adds routes
> locally. In case of routes which are added on behalf of router
> advertisments this flag must not get used as it allows an unlimited
> number of routes getting added remotely.
>
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> [bwh: Backported to 3.2: adjust context]
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> This is untested, but looks almost identical to what you sent for 3.4.
> Please ack/nak.
This looks good to me:
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3.2] ipv6: don't set DST_NOCOUNT for remotely added routes
2014-04-13 23:19 ` David Miller
@ 2014-04-13 23:49 ` Ben Hutchings
0 siblings, 0 replies; 6+ messages in thread
From: Ben Hutchings @ 2014-04-13 23:49 UTC (permalink / raw)
To: David Miller; +Cc: stable, sd, hannes, netdev
[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]
On Sun, 2014-04-13 at 19:19 -0400, David Miller wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> Date: Mon, 14 Apr 2014 00:02:22 +0100
>
> > From: Sabrina Dubroca <sd@queasysnail.net>
> >
> > commit c88507fbad8055297c1d1e21e599f46960cbee39 upstream.
> >
> > DST_NOCOUNT should only be used if an authorized user adds routes
> > locally. In case of routes which are added on behalf of router
> > advertisments this flag must not get used as it allows an unlimited
> > number of routes getting added remotely.
> >
> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > [bwh: Backported to 3.2: adjust context]
> > Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> > ---
> > This is untested, but looks almost identical to what you sent for 3.4.
> > Please ack/nak.
>
> This looks good to me:
>
> Acked-by: David S. Miller <davem@davemloft.net>
Thanks, I've queued this up now.
Ben.
--
Ben Hutchings
I say we take off; nuke the site from orbit. It's the only way to be sure.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread