* [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
@ 2016-07-05 8:18 Vegard Nossum
2016-07-05 10:14 ` Nicolas Dichtel
2016-07-05 19:13 ` David Miller
0 siblings, 2 replies; 6+ messages in thread
From: Vegard Nossum @ 2016-07-05 8:18 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, Nicolas Dichtel
Cc: davem, netdev, Vegard Nossum
If we hit any of the error conditions inside xfrm_dump_sa(), then
xfrm_state_walk_init() never gets called. However, we still call
xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
because the state walk was never initialized properly.
We can fix this by setting cb->args[0] only after we've processed the
first element and checking this before calling xfrm_state_walk_done().
Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
---
net/xfrm/xfrm_user.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index d516845..4fb04ce 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -896,7 +896,8 @@ static int xfrm_dump_sa_done(struct netlink_callback *cb)
struct sock *sk = cb->skb->sk;
struct net *net = sock_net(sk);
- xfrm_state_walk_done(walk, net);
+ if (cb->args[0])
+ xfrm_state_walk_done(walk, net);
return 0;
}
@@ -921,8 +922,6 @@ static int xfrm_dump_sa(struct sk_buff *skb, struct netlink_callback *cb)
u8 proto = 0;
int err;
- cb->args[0] = 1;
-
err = nlmsg_parse(cb->nlh, 0, attrs, XFRMA_MAX,
xfrma_policy);
if (err < 0)
@@ -939,6 +938,7 @@ static int xfrm_dump_sa(struct sk_buff *skb, struct netlink_callback *cb)
proto = nla_get_u8(attrs[XFRMA_PROTO]);
xfrm_state_walk_init(walk, proto, filter);
+ cb->args[0] = 1;
}
(void) xfrm_state_walk(net, walk, dump_one_state, &info);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
2016-07-05 8:18 [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler Vegard Nossum
@ 2016-07-05 10:14 ` Nicolas Dichtel
2016-07-05 19:13 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Nicolas Dichtel @ 2016-07-05 10:14 UTC (permalink / raw)
To: Vegard Nossum, Steffen Klassert, Herbert Xu; +Cc: davem, netdev
Le 05/07/2016 10:18, Vegard Nossum a écrit :
> If we hit any of the error conditions inside xfrm_dump_sa(), then
> xfrm_state_walk_init() never gets called. However, we still call
> xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
> because the state walk was never initialized properly.
>
> We can fix this by setting cb->args[0] only after we've processed the
> first element and checking this before calling xfrm_state_walk_done().
>
> Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Thank you for the fix!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
2016-07-05 8:18 [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler Vegard Nossum
2016-07-05 10:14 ` Nicolas Dichtel
@ 2016-07-05 19:13 ` David Miller
2016-07-06 2:27 ` Herbert Xu
1 sibling, 1 reply; 6+ messages in thread
From: David Miller @ 2016-07-05 19:13 UTC (permalink / raw)
To: vegard.nossum; +Cc: steffen.klassert, herbert, nicolas.dichtel, netdev
From: Vegard Nossum <vegard.nossum@oracle.com>
Date: Tue, 5 Jul 2016 10:18:08 +0200
> If we hit any of the error conditions inside xfrm_dump_sa(), then
> xfrm_state_walk_init() never gets called. However, we still call
> xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
> because the state walk was never initialized properly.
>
> We can fix this by setting cb->args[0] only after we've processed the
> first element and checking this before calling xfrm_state_walk_done().
>
> Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Cc: Steffen Klassert <steffen.klassert@secunet.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
I assume Steffen will pick this up.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
2016-07-05 19:13 ` David Miller
@ 2016-07-06 2:27 ` Herbert Xu
2016-07-10 12:21 ` Steffen Klassert
0 siblings, 1 reply; 6+ messages in thread
From: Herbert Xu @ 2016-07-06 2:27 UTC (permalink / raw)
To: David Miller; +Cc: vegard.nossum, steffen.klassert, nicolas.dichtel, netdev
On Tue, Jul 05, 2016 at 12:13:03PM -0700, David Miller wrote:
> From: Vegard Nossum <vegard.nossum@oracle.com>
> Date: Tue, 5 Jul 2016 10:18:08 +0200
>
> > If we hit any of the error conditions inside xfrm_dump_sa(), then
> > xfrm_state_walk_init() never gets called. However, we still call
> > xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
> > because the state walk was never initialized properly.
> >
> > We can fix this by setting cb->args[0] only after we've processed the
> > first element and checking this before calling xfrm_state_walk_done().
> >
> > Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
> > Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > Cc: Steffen Klassert <steffen.klassert@secunet.com>
> > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
>
> I assume Steffen will pick this up.
I think Steffen said that he is going to be on vacation for two
weeks starting this week.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
2016-07-06 2:27 ` Herbert Xu
@ 2016-07-10 12:21 ` Steffen Klassert
2016-07-19 7:35 ` Steffen Klassert
0 siblings, 1 reply; 6+ messages in thread
From: Steffen Klassert @ 2016-07-10 12:21 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, vegard.nossum, nicolas.dichtel, netdev
On Wed, Jul 06, 2016 at 10:27:29AM +0800, Herbert Xu wrote:
> On Tue, Jul 05, 2016 at 12:13:03PM -0700, David Miller wrote:
> > From: Vegard Nossum <vegard.nossum@oracle.com>
> > Date: Tue, 5 Jul 2016 10:18:08 +0200
> >
> > > If we hit any of the error conditions inside xfrm_dump_sa(), then
> > > xfrm_state_walk_init() never gets called. However, we still call
> > > xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
> > > because the state walk was never initialized properly.
> > >
> > > We can fix this by setting cb->args[0] only after we've processed the
> > > first element and checking this before calling xfrm_state_walk_done().
> > >
> > > Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
> > > Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > > Cc: Steffen Klassert <steffen.klassert@secunet.com>
> > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> >
> > I assume Steffen will pick this up.
>
> I think Steffen said that he is going to be on vacation for two
> weeks starting this week.
Right, I'll take care of it as soon as I'm back.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler
2016-07-10 12:21 ` Steffen Klassert
@ 2016-07-19 7:35 ` Steffen Klassert
0 siblings, 0 replies; 6+ messages in thread
From: Steffen Klassert @ 2016-07-19 7:35 UTC (permalink / raw)
To: Herbert Xu; +Cc: David Miller, vegard.nossum, nicolas.dichtel, netdev
On Sun, Jul 10, 2016 at 02:21:13PM +0200, Steffen Klassert wrote:
> On Wed, Jul 06, 2016 at 10:27:29AM +0800, Herbert Xu wrote:
> > On Tue, Jul 05, 2016 at 12:13:03PM -0700, David Miller wrote:
> > > From: Vegard Nossum <vegard.nossum@oracle.com>
> > > Date: Tue, 5 Jul 2016 10:18:08 +0200
> > >
> > > > If we hit any of the error conditions inside xfrm_dump_sa(), then
> > > > xfrm_state_walk_init() never gets called. However, we still call
> > > > xfrm_state_walk_done() from xfrm_dump_sa_done(), which will crash
> > > > because the state walk was never initialized properly.
> > > >
> > > > We can fix this by setting cb->args[0] only after we've processed the
> > > > first element and checking this before calling xfrm_state_walk_done().
> > > >
> > > > Fixes: d3623099d3 ("ipsec: add support of limited SA dump")
> > > > Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> > > > Cc: Steffen Klassert <steffen.klassert@secunet.com>
> > > > Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> > >
> > > I assume Steffen will pick this up.
> >
> > I think Steffen said that he is going to be on vacation for two
> > weeks starting this week.
>
> Right, I'll take care of it as soon as I'm back.
Now applied to the ipsec tree, thanks a lot!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-19 7:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-05 8:18 [PATCH] xfrm: fix crash in XFRM_MSG_GETSA netlink handler Vegard Nossum
2016-07-05 10:14 ` Nicolas Dichtel
2016-07-05 19:13 ` David Miller
2016-07-06 2:27 ` Herbert Xu
2016-07-10 12:21 ` Steffen Klassert
2016-07-19 7:35 ` Steffen Klassert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).