xfrm: Fix unregister netdevice hang on hardware offload.

When offloading xfrm states to hardware, the offloading
device is attached to the skbs secpath. If a skb is free
is deferred, an unregister netdevice hangs because the
netdevice is still refcounted.

Fix this by removing the netdevice from the xfrm states
when the netdevice is unregistered. To find all xfrm states
that need to be cleared we add another list where skbs
linked to that are unlinked from the lists (deleted)
but not yet freed.

Fixes: d77e38e612a0 ("xfrm: Add an IPsec hardware offloading API")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 77ebf5bcf0b901b7b70875b3ccb5cead14ab1602..7d4c2235252c73d82bb26f8c3d5d77441fed33b8 100644 (file)
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -178,7 +178,10 @@ struct xfrm_state {
                struct hlist_node       gclist;
                struct hlist_node       bydst;
        };
-       struct hlist_node       bysrc;
+       union {
+               struct hlist_node       dev_gclist;
+               struct hlist_node       bysrc;
+       };
        struct hlist_node       byspi;
        struct hlist_node       byseq;
 
@@ -1588,7 +1591,7 @@ void xfrm_state_update_stats(struct net *net);
 static inline void xfrm_dev_state_update_stats(struct xfrm_state *x)
 {
        struct xfrm_dev_offload *xdo = &x->xso;
-       struct net_device *dev = xdo->dev;
+       struct net_device *dev = READ_ONCE(xdo->dev);
 
        if (dev && dev->xfrmdev_ops &&
            dev->xfrmdev_ops->xdo_dev_state_update_stats)
@@ -1946,13 +1949,16 @@ int xfrm_dev_policy_add(struct net *net, struct xfrm_policy *xp,
                        struct xfrm_user_offload *xuo, u8 dir,
                        struct netlink_ext_ack *extack);
 bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x);
+void xfrm_dev_state_delete(struct xfrm_state *x);
+void xfrm_dev_state_free(struct xfrm_state *x);
 
 static inline void xfrm_dev_state_advance_esn(struct xfrm_state *x)
 {
        struct xfrm_dev_offload *xso = &x->xso;
+       struct net_device *dev = READ_ONCE(xso->dev);
 
-       if (xso->dev && xso->dev->xfrmdev_ops->xdo_dev_state_advance_esn)
-               xso->dev->xfrmdev_ops->xdo_dev_state_advance_esn(x);
+       if (dev && dev->xfrmdev_ops->xdo_dev_state_advance_esn)
+               dev->xfrmdev_ops->xdo_dev_state_advance_esn(x);
 }
 
 static inline bool xfrm_dst_offload_ok(struct dst_entry *dst)
@@ -1973,28 +1979,6 @@ static inline bool xfrm_dst_offload_ok(struct dst_entry *dst)
        return false;
 }
 
-static inline void xfrm_dev_state_delete(struct xfrm_state *x)
-{
-       struct xfrm_dev_offload *xso = &x->xso;
-
-       if (xso->dev)
-               xso->dev->xfrmdev_ops->xdo_dev_state_delete(x);
-}
-
-static inline void xfrm_dev_state_free(struct xfrm_state *x)
-{
-       struct xfrm_dev_offload *xso = &x->xso;
-       struct net_device *dev = xso->dev;
-
-       if (dev && dev->xfrmdev_ops) {
-               if (dev->xfrmdev_ops->xdo_dev_state_free)
-                       dev->xfrmdev_ops->xdo_dev_state_free(x);
-               xso->dev = NULL;
-               xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
-               netdev_put(dev, &xso->dev_tracker);
-       }
-}
-
 static inline void xfrm_dev_policy_delete(struct xfrm_policy *x)
 {
        struct xfrm_dev_offload *xdo = &x->xdo;

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 649bb739df0dd672322f09d272d4ba19faf12216..d531d2a1fae28223844d74cc39bc338973c508e0 100644 (file)
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -49,6 +49,7 @@ static struct kmem_cache *xfrm_state_cache __ro_after_init;
 
 static DECLARE_WORK(xfrm_state_gc_work, xfrm_state_gc_task);
 static HLIST_HEAD(xfrm_state_gc_list);
+static HLIST_HEAD(xfrm_state_dev_gc_list);
 
 static inline bool xfrm_state_hold_rcu(struct xfrm_state __rcu *x)
 {
@@ -214,6 +215,7 @@ static DEFINE_SPINLOCK(xfrm_state_afinfo_lock);
 static struct xfrm_state_afinfo __rcu *xfrm_state_afinfo[NPROTO];
 
 static DEFINE_SPINLOCK(xfrm_state_gc_lock);
+static DEFINE_SPINLOCK(xfrm_state_dev_gc_lock);
 
 int __xfrm_state_delete(struct xfrm_state *x);
 
@@ -683,6 +685,40 @@ struct xfrm_state *xfrm_state_alloc(struct net *net)
 }
 EXPORT_SYMBOL(xfrm_state_alloc);
 
+#ifdef CONFIG_XFRM_OFFLOAD
+void xfrm_dev_state_delete(struct xfrm_state *x)
+{
+       struct xfrm_dev_offload *xso = &x->xso;
+       struct net_device *dev = READ_ONCE(xso->dev);
+
+       if (dev) {
+               dev->xfrmdev_ops->xdo_dev_state_delete(x);
+               spin_lock_bh(&xfrm_state_dev_gc_lock);
+               hlist_add_head(&x->dev_gclist, &xfrm_state_dev_gc_list);
+               spin_unlock_bh(&xfrm_state_dev_gc_lock);
+       }
+}
+
+void xfrm_dev_state_free(struct xfrm_state *x)
+{
+       struct xfrm_dev_offload *xso = &x->xso;
+       struct net_device *dev = READ_ONCE(xso->dev);
+
+       if (dev && dev->xfrmdev_ops) {
+               spin_lock_bh(&xfrm_state_dev_gc_lock);
+               if (!hlist_unhashed(&x->dev_gclist))
+                       hlist_del(&x->dev_gclist);
+               spin_unlock_bh(&xfrm_state_dev_gc_lock);
+
+               if (dev->xfrmdev_ops->xdo_dev_state_free)
+                       dev->xfrmdev_ops->xdo_dev_state_free(x);
+               WRITE_ONCE(xso->dev, NULL);
+               xso->type = XFRM_DEV_OFFLOAD_UNSPECIFIED;
+               netdev_put(dev, &xso->dev_tracker);
+       }
+}
+#endif
+
 void __xfrm_state_destroy(struct xfrm_state *x, bool sync)
 {
        WARN_ON(x->km.state != XFRM_STATE_DEAD);
@@ -848,6 +884,9 @@ EXPORT_SYMBOL(xfrm_state_flush);
 
 int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_valid)
 {
+       struct xfrm_state *x;
+       struct hlist_node *tmp;
+       struct xfrm_dev_offload *xso;
        int i, err = 0, cnt = 0;
 
        spin_lock_bh(&net->xfrm.xfrm_state_lock);
@@ -857,8 +896,6 @@ int xfrm_dev_state_flush(struct net *net, struct net_device *dev, bool task_vali
 
        err = -ESRCH;
        for (i = 0; i <= net->xfrm.state_hmask; i++) {
-               struct xfrm_state *x;
-               struct xfrm_dev_offload *xso;
 restart:
                hlist_for_each_entry(x, net->xfrm.state_bydst+i, bydst) {
                        xso = &x->xso;
@@ -868,6 +905,8 @@ restart:
                                spin_unlock_bh(&net->xfrm.xfrm_state_lock);
 
                                err = xfrm_state_delete(x);
+                               xfrm_dev_state_free(x);
+
                                xfrm_audit_state_delete(x, err ? 0 : 1,
                                                        task_valid);
                                xfrm_state_put(x);
@@ -884,6 +923,24 @@ restart:
 
 out:
        spin_unlock_bh(&net->xfrm.xfrm_state_lock);
+
+       spin_lock_bh(&xfrm_state_dev_gc_lock);
+restart_gc:
+       hlist_for_each_entry_safe(x, tmp, &xfrm_state_dev_gc_list, dev_gclist) {
+               xso = &x->xso;
+
+               if (xso->dev == dev) {
+                       spin_unlock_bh(&xfrm_state_dev_gc_lock);
+                       xfrm_dev_state_free(x);
+                       spin_lock_bh(&xfrm_state_dev_gc_lock);
+                       goto restart_gc;
+               }
+
+       }
+       spin_unlock_bh(&xfrm_state_dev_gc_lock);
+
+       xfrm_flush_gc();
+
        return err;
 }
 EXPORT_SYMBOL(xfrm_dev_state_flush);