On 12/1/2017 11:47 AM, Shannon Nelson wrote:
On 11/28/2017 9:55 AM, av...@mellanox.com wrote:
From: Aviv Heller <av...@mellanox.com>
Adding the state to the offload device prior to replay init in
xfrm_state_construct() will result in NULL dereference if a matching
ESP packet is received in between.
In order to inhibit driver offload logic from processing the state's
packets prior to the xfrm_state object being completely initialized and
added to the SADBs, a new activate() operation was added to inform the
driver the aforementioned conditions have been met.
Are there also conditions where you would want to temporarily
deactivate, or pause, the incoming driver offload, followed then by
another activate?
sln
Instead of setting up a half-ready state that needs the activate()
operation to finish, can we instead just move the xfrm_dev_state_add()
call to after the xfrm_init_replay()? Especially since this really only
makes sense for the inbound, and makes no sense for the outbound path.
sln
Signed-off-by: Aviv Heller <av...@mellanox.com>
Signed-off-by: Yossi Kuperman <yoss...@mellanox.com>
---
v1 -> v2:
- Separate to state addition and then activation, instead
of relocating dev state addition call.
---
include/linux/netdevice.h | 1 +
include/net/xfrm.h | 12 ++++++++++++
net/xfrm/xfrm_user.c | 5 +++++
3 files changed, 18 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2eaac7d..c6ca356 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -819,6 +819,7 @@ struct netdev_xdp {
#ifdef CONFIG_XFRM_OFFLOAD
struct xfrmdev_ops {
int (*xdo_dev_state_add) (struct xfrm_state *x);
+ void (*xdo_dev_state_activate) (struct xfrm_state *x);
void (*xdo_dev_state_delete) (struct xfrm_state *x);
void (*xdo_dev_state_free) (struct xfrm_state *x);
bool (*xdo_dev_offload_ok) (struct sk_buff *skb,
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index e015e16..324374e 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1877,6 +1877,14 @@ static inline bool xfrm_dst_offload_ok(struct
dst_entry *dst)
return false;
}
+static inline void xfrm_dev_state_activate(struct xfrm_state *x)
+{
+ struct xfrm_state_offload *xso = &x->xso;
+
+ if (xso->dev && xso->dev->xfrmdev_ops->xdo_dev_state_activate)
+ xso->dev->xfrmdev_ops->xdo_dev_state_activate(x);
+}
+
static inline void xfrm_dev_state_delete(struct xfrm_state *x)
{
struct xfrm_state_offload *xso = &x->xso;
@@ -1907,6 +1915,10 @@ static inline int xfrm_dev_state_add(struct net
*net, struct xfrm_state *x, stru
return 0;
}
+static inline void xfrm_dev_state_activate(struct xfrm_state *x)
+{
+}
+
static inline void xfrm_dev_state_delete(struct xfrm_state *x)
{
}
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index e44a0fe..d06f579 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -662,6 +662,11 @@ static int xfrm_add_sa(struct sk_buff *skb,
struct nlmsghdr *nlh,
goto out;
}
+ spin_lock_bh(&x->lock);
+ if (x->km.state == XFRM_STATE_VALID)
+ xfrm_dev_state_activate(x);
+ spin_unlock_bh(&x->lock);
+
c.seq = nlh->nlmsg_seq;
c.portid = nlh->nlmsg_pid;
c.event = nlh->nlmsg_type;
