* [PATCH 1/3] xfrm: prevent ipcomp scratch buffer race condition
2013-11-01 8:21 pull request (net): ipsec 2013-11-01 Steffen Klassert
@ 2013-11-01 8:21 ` Steffen Klassert
2013-11-01 8:21 ` [PATCH 2/3] xfrm: Increase the garbage collector threshold Steffen Klassert
` (2 subsequent siblings)
3 siblings, 0 replies; 17+ messages in thread
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Michal Kubecek <mkubecek@suse.cz>
In ipcomp_compress(), sortirq is enabled too early, allowing the
per-cpu scratch buffer to be rewritten by ipcomp_decompress()
(called on the same CPU in softirq context) between populating
the buffer and copying the compressed data to the skb.
v2: as pointed out by Steffen Klassert, if we also move the
local_bh_disable() before reading the per-cpu pointers, we can
get rid of get_cpu()/put_cpu().
v3: removed ipcomp_decompress part (as explained by Herbert Xu,
it cannot be called from process context), get rid of cpu
variable (thanks to Eric Dumazet)
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_ipcomp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c
index 2906d52..3be02b6 100644
--- a/net/xfrm/xfrm_ipcomp.c
+++ b/net/xfrm/xfrm_ipcomp.c
@@ -141,14 +141,14 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
const int plen = skb->len;
int dlen = IPCOMP_SCRATCH_SIZE;
u8 *start = skb->data;
- const int cpu = get_cpu();
- u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu);
- struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu);
+ struct crypto_comp *tfm;
+ u8 *scratch;
int err;
local_bh_disable();
+ scratch = *this_cpu_ptr(ipcomp_scratches);
+ tfm = *this_cpu_ptr(ipcd->tfms);
err = crypto_comp_compress(tfm, start, plen, scratch, &dlen);
- local_bh_enable();
if (err)
goto out;
@@ -158,13 +158,13 @@ static int ipcomp_compress(struct xfrm_state *x, struct sk_buff *skb)
}
memcpy(start + sizeof(struct ip_comp_hdr), scratch, dlen);
- put_cpu();
+ local_bh_enable();
pskb_trim(skb, dlen + sizeof(struct ip_comp_hdr));
return 0;
out:
- put_cpu();
+ local_bh_enable();
return err;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-11-01 8:21 pull request (net): ipsec 2013-11-01 Steffen Klassert
2013-11-01 8:21 ` [PATCH 1/3] xfrm: prevent ipcomp scratch buffer race condition Steffen Klassert
@ 2013-11-01 8:21 ` Steffen Klassert
2013-12-10 14:38 ` Maxime Bizon
2013-11-01 8:21 ` [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions Steffen Klassert
2013-11-02 5:22 ` pull request (net): ipsec 2013-11-01 David Miller
3 siblings, 1 reply; 17+ messages in thread
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
With the removal of the routing cache, we lost the
option to tweak the garbage collector threshold
along with the maximum routing cache size. So git
commit 703fb94ec ("xfrm: Fix the gc threshold value
for ipv4") moved back to a static threshold.
It turned out that the current threshold before we
start garbage collecting is much to small for some
workloads, so increase it from 1024 to 32768. This
means that we start the garbage collector if we have
more than 32768 dst entries in the system and refuse
new allocations if we are above 65536.
Reported-by: Wolfgang Walter <linux@stwm.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 2 +-
net/ipv6/xfrm6_policy.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index ccde542..4764ee4 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -236,7 +236,7 @@ static struct dst_ops xfrm4_dst_ops = {
.destroy = xfrm4_dst_destroy,
.ifdown = xfrm4_dst_ifdown,
.local_out = __ip_local_out,
- .gc_thresh = 1024,
+ .gc_thresh = 32768,
};
static struct xfrm_policy_afinfo xfrm4_policy_afinfo = {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 08ed277..dd503a3 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -285,7 +285,7 @@ static struct dst_ops xfrm6_dst_ops = {
.destroy = xfrm6_dst_destroy,
.ifdown = xfrm6_dst_ifdown,
.local_out = __ip6_local_out,
- .gc_thresh = 1024,
+ .gc_thresh = 32768,
};
static struct xfrm_policy_afinfo xfrm6_policy_afinfo = {
--
1.7.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-11-01 8:21 ` [PATCH 2/3] xfrm: Increase the garbage collector threshold Steffen Klassert
@ 2013-12-10 14:38 ` Maxime Bizon
2013-12-13 10:12 ` Steffen Klassert
0 siblings, 1 reply; 17+ messages in thread
From: Maxime Bizon @ 2013-12-10 14:38 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, Herbert Xu, netdev
On Fri, 2013-11-01 at 09:21 +0100, Steffen Klassert wrote:
Hello Steffen,
> It turned out that the current threshold before we
> start garbage collecting is much to small for some
> workloads, so increase it from 1024 to 32768. This
> means that we start the garbage collector if we have
> more than 32768 dst entries in the system and refuse
> new allocations if we are above 65536.
sorry to dig an old thread, but could you please explain the lifetime of
those dst entries ?
After basic tests it seems the ipv4 dst entry needs to be released first
before the xfrm dst can be released by gc.
--
Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-12-10 14:38 ` Maxime Bizon
@ 2013-12-13 10:12 ` Steffen Klassert
2013-12-13 16:13 ` Maxime Bizon
0 siblings, 1 reply; 17+ messages in thread
From: Steffen Klassert @ 2013-12-13 10:12 UTC (permalink / raw)
To: Maxime Bizon; +Cc: David Miller, Herbert Xu, netdev
On Tue, Dec 10, 2013 at 03:38:43PM +0100, Maxime Bizon wrote:
>
> On Fri, 2013-11-01 at 09:21 +0100, Steffen Klassert wrote:
>
> Hello Steffen,
>
> > It turned out that the current threshold before we
> > start garbage collecting is much to small for some
> > workloads, so increase it from 1024 to 32768. This
> > means that we start the garbage collector if we have
> > more than 32768 dst entries in the system and refuse
> > new allocations if we are above 65536.
>
> sorry to dig an old thread, but could you please explain the lifetime of
> those dst entries ?
>
> After basic tests it seems the ipv4 dst entry needs to be released first
> before the xfrm dst can be released by gc.
>
Can you please be a bit more precise with your problem description?
This patch changes only the number of cache entries before
we start garbage collecting. It does not change anything
on the garbage collector itself.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-12-13 10:12 ` Steffen Klassert
@ 2013-12-13 16:13 ` Maxime Bizon
2013-12-16 11:51 ` Steffen Klassert
0 siblings, 1 reply; 17+ messages in thread
From: Maxime Bizon @ 2013-12-13 16:13 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, Herbert Xu, netdev
On Fri, 2013-12-13 at 11:12 +0100, Steffen Klassert wrote:
> Can you please be a bit more precise with your problem description?
yes sorry I wasn't clear.
I have a problem with an even simpler workload that the one using apache
bench in the original bug report.
I am using ipsec transport mode between two hosts and just run this on
one side:
while :; do wget -O /dev/null http://remote_host/; done
I was surprised to see it fails after only 1024 requests (ENOBUF on
connect), and how long I had to wait to be able to do new requests.
After debugging I saw that the xfrm gc was called but was not able to
release anything.
after running "ip route flush cache", which forces all ipv4 dst entries
to be released, suddenly the xfrm gc had something to free, and xfrm
entry count went to zero.
So if it is correct that once a ipv4 dst entry exists, the xfrm entry
cannot be gc-ed, then we need to make sure we allow more xfrm entries to
be allocated than ipv4 dst.
> This patch changes only the number of cache entries before
> we start garbage collecting. It does not change anything
> on the garbage collector itself.
Yes my point was that it just hides another underlying problem.
--
Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-12-13 16:13 ` Maxime Bizon
@ 2013-12-16 11:51 ` Steffen Klassert
2013-12-16 14:29 ` Maxime Bizon
0 siblings, 1 reply; 17+ messages in thread
From: Steffen Klassert @ 2013-12-16 11:51 UTC (permalink / raw)
To: Maxime Bizon; +Cc: David Miller, Herbert Xu, netdev
On Fri, Dec 13, 2013 at 05:13:29PM +0100, Maxime Bizon wrote:
>
> On Fri, 2013-12-13 at 11:12 +0100, Steffen Klassert wrote:
>
> > Can you please be a bit more precise with your problem description?
>
> yes sorry I wasn't clear.
>
> I have a problem with an even simpler workload that the one using apache
> bench in the original bug report.
>
> I am using ipsec transport mode between two hosts and just run this on
> one side:
>
> while :; do wget -O /dev/null http://remote_host/; done
>
> I was surprised to see it fails after only 1024 requests (ENOBUF on
> connect), and how long I had to wait to be able to do new requests.
>
> After debugging I saw that the xfrm gc was called but was not able to
> release anything.
>
> after running "ip route flush cache", which forces all ipv4 dst entries
> to be released, suddenly the xfrm gc had something to free, and xfrm
> entry count went to zero.
Well, "ip route flush cache" bumps the rt_genid what marks all
routes as invalid. The xfrm garbage collector will notice this
on the next run and deletes all invalid cached routes.
Garbage collecting is different from flushing, it only removes
stale routes and keeps everything that was used recently. That's
the whole point of this caching, we cache recently used IPsec
routes to avoid a slow path lookup.
>
> So if it is correct that once a ipv4 dst entry exists, the xfrm entry
> cannot be gc-ed, then we need to make sure we allow more xfrm entries to
> be allocated than ipv4 dst.
No, we don't cache plain ipv4 routes, we cache only IPsec routes.
The original ipv4 dst_entry is cached together with the xfrm dst_entry
as a xfrm bundle at the IPsec flow cache, so it does not make sense to
ensure something like that.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/3] xfrm: Increase the garbage collector threshold
2013-12-16 11:51 ` Steffen Klassert
@ 2013-12-16 14:29 ` Maxime Bizon
0 siblings, 0 replies; 17+ messages in thread
From: Maxime Bizon @ 2013-12-16 14:29 UTC (permalink / raw)
To: Steffen Klassert; +Cc: David Miller, Herbert Xu, netdev
On Mon, 2013-12-16 at 12:51 +0100, Steffen Klassert wrote:
> > while :; do wget -O /dev/null http://remote_host/; done
> >
> > I was surprised to see it fails after only 1024 requests (ENOBUF on
> > connect), and how long I had to wait to be able to do new requests.
> >
> > After debugging I saw that the xfrm gc was called but was not able to
> > release anything.
> >
> > after running "ip route flush cache", which forces all ipv4 dst entries
> > to be released, suddenly the xfrm gc had something to free, and xfrm
> > entry count went to zero.
>
> Well, "ip route flush cache" bumps the rt_genid what marks all
> routes as invalid. The xfrm garbage collector will notice this
> on the next run and deletes all invalid cached routes.
>
> Garbage collecting is different from flushing, it only removes
> stale routes and keeps everything that was used recently. That's
> the whole point of this caching, we cache recently used IPsec
> routes to avoid a slow path lookup.
ok I don't know this part of the code at all.
but if it is only a *cache*, then it should not limit any new
allocations for the reason that the cache is full, it should take slow
path instead.
--
Maxime
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-11-01 8:21 pull request (net): ipsec 2013-11-01 Steffen Klassert
2013-11-01 8:21 ` [PATCH 1/3] xfrm: prevent ipcomp scratch buffer race condition Steffen Klassert
2013-11-01 8:21 ` [PATCH 2/3] xfrm: Increase the garbage collector threshold Steffen Klassert
@ 2013-11-01 8:21 ` Steffen Klassert
2013-11-24 14:27 ` Matthias Schiffer
2013-11-02 5:22 ` pull request (net): ipsec 2013-11-01 David Miller
3 siblings, 1 reply; 17+ messages in thread
From: Steffen Klassert @ 2013-11-01 8:21 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
On some codepaths the skb does not have a dst entry
when xfrm_decode_session() is called. So check for
a valid skb_dst() before dereferencing the device
interface index. We use 0 as the device index if
there is no valid skb_dst(), or at reverse decoding
we use skb_iif as device interface index.
Bug was introduced with git commit bafd4bd4dc
("xfrm: Decode sessions with output interface.").
Reported-by: Meelis Roos <mroos@linux.ee>
Tested-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/xfrm4_policy.c | 6 +++++-
net/ipv6/xfrm6_policy.c | 6 +++++-
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4764ee4..e1a6393 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -104,10 +104,14 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
const struct iphdr *iph = ip_hdr(skb);
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
struct flowi4 *fl4 = &fl->u.ip4;
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl4, 0, sizeof(struct flowi4));
fl4->flowi4_mark = skb->mark;
- fl4->flowi4_oif = skb_dst(skb)->dev->ifindex;
+ fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
if (!ip_is_fragment(iph)) {
switch (iph->protocol) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index dd503a3..5f8e128 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -135,10 +135,14 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl6, 0, sizeof(struct flowi6));
fl6->flowi6_mark = skb->mark;
- fl6->flowi6_oif = skb_dst(skb)->dev->ifindex;
+ fl6->flowi6_oif = reverse ? skb->skb_iif : oif;
fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-11-01 8:21 ` [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions Steffen Klassert
@ 2013-11-24 14:27 ` Matthias Schiffer
2013-12-01 4:19 ` Matthias Schiffer
0 siblings, 1 reply; 17+ messages in thread
From: Matthias Schiffer @ 2013-11-24 14:27 UTC (permalink / raw)
To: Steffen Klassert, David Miller; +Cc: Herbert Xu, netdev
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On 11/01/2013 09:21 AM, Steffen Klassert wrote:
> On some codepaths the skb does not have a dst entry
> when xfrm_decode_session() is called. So check for
> a valid skb_dst() before dereferencing the device
> interface index. We use 0 as the device index if
> there is no valid skb_dst(), or at reverse decoding
> we use skb_iif as device interface index.
>
> Bug was introduced with git commit bafd4bd4dc
> ("xfrm: Decode sessions with output interface.").
>
> Reported-by: Meelis Roos <mroos@linux.ee>
> Tested-by: Meelis Roos <mroos@linux.ee>
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Please queue this for 3.12-stable, I've seen this bug trigger a panic
when heavily using Bittorrent (screenshot:
http://i.imgur.com/OIKVccM.jpg )
Thanks,
Matthias
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-11-24 14:27 ` Matthias Schiffer
@ 2013-12-01 4:19 ` Matthias Schiffer
2013-12-02 1:35 ` David Miller
0 siblings, 1 reply; 17+ messages in thread
From: Matthias Schiffer @ 2013-12-01 4:19 UTC (permalink / raw)
To: Steffen Klassert, David Miller; +Cc: Herbert Xu, netdev
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On 11/24/2013 03:27 PM, Matthias Schiffer wrote:
> On 11/01/2013 09:21 AM, Steffen Klassert wrote:
>> On some codepaths the skb does not have a dst entry
>> when xfrm_decode_session() is called. So check for
>> a valid skb_dst() before dereferencing the device
>> interface index. We use 0 as the device index if
>> there is no valid skb_dst(), or at reverse decoding
>> we use skb_iif as device interface index.
>>
>> Bug was introduced with git commit bafd4bd4dc
>> ("xfrm: Decode sessions with output interface.").
>>
>> Reported-by: Meelis Roos <mroos@linux.ee>
>> Tested-by: Meelis Roos <mroos@linux.ee>
>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>
> Please queue this for 3.12-stable, I've seen this bug trigger a panic
> when heavily using Bittorrent (screenshot:
> http://i.imgur.com/OIKVccM.jpg )
>
> Thanks,
> Matthias
>
Ping? Several people I know have hit this panic with 3.12.x at least
once or twice by now during heavy IPv6 usage.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-12-01 4:19 ` Matthias Schiffer
@ 2013-12-02 1:35 ` David Miller
2013-12-03 9:56 ` Steffen Klassert
0 siblings, 1 reply; 17+ messages in thread
From: David Miller @ 2013-12-02 1:35 UTC (permalink / raw)
To: mschiffer; +Cc: steffen.klassert, herbert, netdev
From: Matthias Schiffer <mschiffer@universe-factory.net>
Date: Sun, 01 Dec 2013 05:19:53 +0100
> On 11/24/2013 03:27 PM, Matthias Schiffer wrote:
>> On 11/01/2013 09:21 AM, Steffen Klassert wrote:
>>> On some codepaths the skb does not have a dst entry
>>> when xfrm_decode_session() is called. So check for
>>> a valid skb_dst() before dereferencing the device
>>> interface index. We use 0 as the device index if
>>> there is no valid skb_dst(), or at reverse decoding
>>> we use skb_iif as device interface index.
>>>
>>> Bug was introduced with git commit bafd4bd4dc
>>> ("xfrm: Decode sessions with output interface.").
>>>
>>> Reported-by: Meelis Roos <mroos@linux.ee>
>>> Tested-by: Meelis Roos <mroos@linux.ee>
>>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>>
>> Please queue this for 3.12-stable, I've seen this bug trigger a panic
>> when heavily using Bittorrent (screenshot:
>> http://i.imgur.com/OIKVccM.jpg )
>>
>> Thanks,
>> Matthias
>>
>
> Ping? Several people I know have hit this panic with 3.12.x at least
> once or twice by now during heavy IPv6 usage.
Steffen submitted it to stable@vger.kernel.org for inclusion on Monday
November 25th.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-12-02 1:35 ` David Miller
@ 2013-12-03 9:56 ` Steffen Klassert
2013-12-03 10:37 ` Wolfgang Walter
2013-12-05 21:35 ` David Miller
0 siblings, 2 replies; 17+ messages in thread
From: Steffen Klassert @ 2013-12-03 9:56 UTC (permalink / raw)
To: David Miller; +Cc: mschiffer, herbert, netdev, stable
Ccing stable.
On Sun, Dec 01, 2013 at 08:35:06PM -0500, David Miller wrote:
> From: Matthias Schiffer <mschiffer@universe-factory.net>
> Date: Sun, 01 Dec 2013 05:19:53 +0100
>
> > On 11/24/2013 03:27 PM, Matthias Schiffer wrote:
> >> On 11/01/2013 09:21 AM, Steffen Klassert wrote:
> >>> On some codepaths the skb does not have a dst entry
> >>> when xfrm_decode_session() is called. So check for
> >>> a valid skb_dst() before dereferencing the device
> >>> interface index. We use 0 as the device index if
> >>> there is no valid skb_dst(), or at reverse decoding
> >>> we use skb_iif as device interface index.
> >>>
> >>> Bug was introduced with git commit bafd4bd4dc
> >>> ("xfrm: Decode sessions with output interface.").
> >>>
> >>> Reported-by: Meelis Roos <mroos@linux.ee>
> >>> Tested-by: Meelis Roos <mroos@linux.ee>
> >>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> >>
> >> Please queue this for 3.12-stable, I've seen this bug trigger a panic
> >> when heavily using Bittorrent (screenshot:
> >> http://i.imgur.com/OIKVccM.jpg )
> >>
> >> Thanks,
> >> Matthias
> >>
> >
> > Ping? Several people I know have hit this panic with 3.12.x at least
> > once or twice by now during heavy IPv6 usage.
>
> Steffen submitted it to stable@vger.kernel.org for inclusion on Monday
> November 25th.
It apparently did not make it into v3.12.2 and it is not in the v3.12.3
review included. So I wonder if I need to do anything additional to
get it included. It is the first time that I did a stable submission
myself.
I tried to follow the instructions at Documentation/stable_kernel_rules.txt
but maybe I've missed something.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-12-03 9:56 ` Steffen Klassert
@ 2013-12-03 10:37 ` Wolfgang Walter
2013-12-03 10:41 ` Hannes Frederic Sowa
2013-12-05 21:35 ` David Miller
1 sibling, 1 reply; 17+ messages in thread
From: Wolfgang Walter @ 2013-12-03 10:37 UTC (permalink / raw)
To: Steffen Klassert, netdev; +Cc: David Miller, mschiffer, stable
Am Dienstag, 3. Dezember 2013, 10:56:45 schrieb Steffen Klassert:
> Ccing stable.
>
> On Sun, Dec 01, 2013 at 08:35:06PM -0500, David Miller wrote:
> > From: Matthias Schiffer <mschiffer@universe-factory.net>
> > Date: Sun, 01 Dec 2013 05:19:53 +0100
> >
> > > On 11/24/2013 03:27 PM, Matthias Schiffer wrote:
> > >> On 11/01/2013 09:21 AM, Steffen Klassert wrote:
> > >>> On some codepaths the skb does not have a dst entry
> > >>> when xfrm_decode_session() is called. So check for
> > >>> a valid skb_dst() before dereferencing the device
> > >>> interface index. We use 0 as the device index if
> > >>> there is no valid skb_dst(), or at reverse decoding
> > >>> we use skb_iif as device interface index.
> > >>>
> > >>> Bug was introduced with git commit bafd4bd4dc
> > >>> ("xfrm: Decode sessions with output interface.").
> > >>>
> > >>> Reported-by: Meelis Roos <mroos@linux.ee>
> > >>> Tested-by: Meelis Roos <mroos@linux.ee>
> > >>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > >>
> > >> Please queue this for 3.12-stable, I've seen this bug trigger a panic
> > >> when heavily using Bittorrent (screenshot:
> > >> http://i.imgur.com/OIKVccM.jpg )
> > >>
> > >> Thanks,
> > >> Matthias
> > >
> > > Ping? Several people I know have hit this panic with 3.12.x at least
> > > once or twice by now during heavy IPv6 usage.
> >
> > Steffen submitted it to stable@vger.kernel.org for inclusion on Monday
> > November 25th.
>
> It apparently did not make it into v3.12.2 and it is not in the v3.12.3
> review included. So I wonder if I need to do anything additional to
> get it included. It is the first time that I did a stable submission
> myself.
>
> I tried to follow the instructions at Documentation/stable_kernel_rules.txt
> but maybe I've missed something.
There is another weakness with ipv6 in 3.12.x which - under certain
circumstances - can be exploited remotely:
ipv6 fragmentation-related panic in netfilter
I would be very glad if this would make it into 3.12 stable soon.
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-12-03 10:37 ` Wolfgang Walter
@ 2013-12-03 10:41 ` Hannes Frederic Sowa
0 siblings, 0 replies; 17+ messages in thread
From: Hannes Frederic Sowa @ 2013-12-03 10:41 UTC (permalink / raw)
To: Wolfgang Walter; +Cc: Steffen Klassert, netdev, David Miller, mschiffer, stable
On Tue, Dec 03, 2013 at 11:37:04AM +0100, Wolfgang Walter wrote:
> Am Dienstag, 3. Dezember 2013, 10:56:45 schrieb Steffen Klassert:
> > Ccing stable.
> >
> > On Sun, Dec 01, 2013 at 08:35:06PM -0500, David Miller wrote:
> > > From: Matthias Schiffer <mschiffer@universe-factory.net>
> > > Date: Sun, 01 Dec 2013 05:19:53 +0100
> > >
> > > > On 11/24/2013 03:27 PM, Matthias Schiffer wrote:
> > > >> On 11/01/2013 09:21 AM, Steffen Klassert wrote:
> > > >>> On some codepaths the skb does not have a dst entry
> > > >>> when xfrm_decode_session() is called. So check for
> > > >>> a valid skb_dst() before dereferencing the device
> > > >>> interface index. We use 0 as the device index if
> > > >>> there is no valid skb_dst(), or at reverse decoding
> > > >>> we use skb_iif as device interface index.
> > > >>>
> > > >>> Bug was introduced with git commit bafd4bd4dc
> > > >>> ("xfrm: Decode sessions with output interface.").
> > > >>>
> > > >>> Reported-by: Meelis Roos <mroos@linux.ee>
> > > >>> Tested-by: Meelis Roos <mroos@linux.ee>
> > > >>> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > > >>
> > > >> Please queue this for 3.12-stable, I've seen this bug trigger a panic
> > > >> when heavily using Bittorrent (screenshot:
> > > >> http://i.imgur.com/OIKVccM.jpg )
> > > >>
> > > >> Thanks,
> > > >> Matthias
> > > >
> > > > Ping? Several people I know have hit this panic with 3.12.x at least
> > > > once or twice by now during heavy IPv6 usage.
> > >
> > > Steffen submitted it to stable@vger.kernel.org for inclusion on Monday
> > > November 25th.
> >
> > It apparently did not make it into v3.12.2 and it is not in the v3.12.3
> > review included. So I wonder if I need to do anything additional to
> > get it included. It is the first time that I did a stable submission
> > myself.
> >
> > I tried to follow the instructions at Documentation/stable_kernel_rules.txt
> > but maybe I've missed something.
>
> There is another weakness with ipv6 in 3.12.x which - under certain
> circumstances - can be exploited remotely:
>
> ipv6 fragmentation-related panic in netfilter
>
> I would be very glad if this would make it into 3.12 stable soon.
These patches are already queued up for stable by David:
http://patchwork.ozlabs.org/bundle/davem/stable/?state=*
Greetings,
Hannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions
2013-12-03 9:56 ` Steffen Klassert
2013-12-03 10:37 ` Wolfgang Walter
@ 2013-12-05 21:35 ` David Miller
1 sibling, 0 replies; 17+ messages in thread
From: David Miller @ 2013-12-05 21:35 UTC (permalink / raw)
To: steffen.klassert; +Cc: mschiffer, herbert, netdev, stable
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 3 Dec 2013 10:56:45 +0100
> It apparently did not make it into v3.12.2 and it is not in the v3.12.3
> review included. So I wonder if I need to do anything additional to
> get it included. It is the first time that I did a stable submission
> myself.
I'm taking care of this myself now, don't worry any more about it.
Thanks.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: pull request (net): ipsec 2013-11-01
2013-11-01 8:21 pull request (net): ipsec 2013-11-01 Steffen Klassert
` (2 preceding siblings ...)
2013-11-01 8:21 ` [PATCH 3/3] xfrm: Fix null pointer dereference when decoding sessions Steffen Klassert
@ 2013-11-02 5:22 ` David Miller
3 siblings, 0 replies; 17+ messages in thread
From: David Miller @ 2013-11-02 5:22 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 1 Nov 2013 09:21:44 +0100
> 1) Fix a possible race on ipcomp scratch buffers because
> of too early enabled siftirqs. From Michal Kubecek.
>
> 2) The current xfrm garbage collector threshold is too small
> for some workloads, resulting in bad performance on these
> workloads. Increase the threshold from 1024 to 32768.
>
> 3) Some codepaths might not have a dst_entry attached to the
> skb when calling xfrm_decode_session(). So add a check
> to prevent a null pointer dereference in this case.
>
> Please pull or let me know if there are problems.
Pulled, thanks a lot Steffen!
^ permalink raw reply [flat|nested] 17+ messages in thread
