2018-01-17 13:53:03

by Yossi Kuperman

Subject: [PATCH net] xfrm: Add SA to hardware at the end of xfrm_state_construct()

From: Yossi Kuperman <[email protected]>

Current code configures the hardware with a new SA before the state has been
fully initialized. During this time interval, an incoming ESP packet can cause
a crash due to a NULL dereference. More specifically, xfrm_input() considers
the packet as valid, and yet, anti-replay mechanism is not initialized.

Move hardware configuration to the end of xfrm_state_construct(), and mark
the state as valid once the SA is fully initialized.

Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Aviad Yehezkel <[email protected]>
Signed-off-by: Aviv Heller <[email protected]>
Signed-off-by: Yossi Kuperman <[email protected]>
---
net/xfrm/xfrm_state.c | 10 +++++++---
net/xfrm/xfrm_user.c | 18 +++++++++++-------
2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index cc4c519..9750233 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2272,8 +2272,6 @@ int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload)
goto error;
}

- x->km.state = XFRM_STATE_VALID;
-
error:
return err;
}
@@ -2282,7 +2280,13 @@ EXPORT_SYMBOL(__xfrm_init_state);

int xfrm_init_state(struct xfrm_state *x)
{
- return __xfrm_init_state(x, true, false);
+ int err;
+
+ err = __xfrm_init_state(x, true, false);
+ if (!err)
+ x->km.state = XFRM_STATE_VALID;
+
+ return err;
}

EXPORT_SYMBOL(xfrm_init_state);
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index bdb48e5..7f52b8e 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -598,13 +598,6 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
goto error;
}

- if (attrs[XFRMA_OFFLOAD_DEV]) {
- err = xfrm_dev_state_add(net, x,
- nla_data(attrs[XFRMA_OFFLOAD_DEV]));
- if (err)
- goto error;
- }
-
if ((err = xfrm_alloc_replay_state_esn(&x->replay_esn, &x->preplay_esn,
attrs[XFRMA_REPLAY_ESN_VAL])))
goto error;
@@ -620,6 +613,14 @@ static struct xfrm_state *xfrm_state_construct(struct net *net,
/* override default values from above */
xfrm_update_ae_params(x, attrs, 0);

+ /* configure the hardware if offload is requested */
+ if (attrs[XFRMA_OFFLOAD_DEV]) {
+ err = xfrm_dev_state_add(net, x,
+ nla_data(attrs[XFRMA_OFFLOAD_DEV]));
+ if (err)
+ goto error;
+ }
+
return x;

error:
@@ -662,6 +663,9 @@ static int xfrm_add_sa(struct sk_buff *skb, struct nlmsghdr *nlh,
goto out;
}

+ if (x->km.state == XFRM_STATE_VOID)
+ x->km.state = XFRM_STATE_VALID;
+
c.seq = nlh->nlmsg_seq;
c.portid = nlh->nlmsg_pid;
c.event = nlh->nlmsg_type;
--
2.8.1

2018-01-19 10:06:47

by Steffen Klassert

Subject: Re: [PATCH net] xfrm: Add SA to hardware at the end of xfrm_state_construct()

On Wed, Jan 17, 2018 at 03:52:41PM +0200, [email protected] wrote:
> From: Yossi Kuperman <[email protected]>
>
> Current code configures the hardware with a new SA before the state has been
> fully initialized. During this time interval, an incoming ESP packet can cause
> a crash due to a NULL dereference. More specifically, xfrm_input() considers
> the packet as valid, and yet, anti-replay mechanism is not initialized.
>
> Move hardware configuration to the end of xfrm_state_construct(), and mark
> the state as valid once the SA is fully initialized.
>
> Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
> Signed-off-by: Aviad Yehezkel <[email protected]>
> Signed-off-by: Aviv Heller <[email protected]>
> Signed-off-by: Yossi Kuperman <[email protected]>

Applied, thanks Yossi!