The fixed commit placed mutex_lock() inside spin_lock_bh(), which triggers
a warning:

  BUG: sleeping function called from invalid context at...

Fix this by moving the IPsec deletion operation to bond_ipsec_free_sa,
which is not held by spin_lock_bh().

Additionally, delete the IPsec list in bond_ipsec_del_sa_all() when the
XFRM state is DEAD to prevent xdo_dev_state_free() from being triggered
again in bond_ipsec_free_sa().

For bond_ipsec_free_sa(), there are now three conditions:

  1. if (!slave): When no active device exists.
  2. if (!xs->xso.real_dev): When xdo_dev_state_add() fails.
  3. if (xs->xso.real_dev != real_dev): When an xs has already been freed
     by bond_ipsec_del_sa_all() due to migration, and the active slave has
     changed to a new device. At the same time, the xs is marked as DEAD
     due to the XFRM entry is removed, triggering xfrm_state_gc_task() and
     bond_ipsec_free_sa().

In all three cases, xdo_dev_state_free() should not be called, only xs
should be removed from bond->ipsec list.

Fixes: 2aeeef906d5a ("bonding: change ipsec_lock from spin lock to mutex")
Reported-by: Jakub Kicinski <[email protected]>
Closes: https://lore.kernel.org/netdev/[email protected]
Suggested-by: Cosmin Ratiu <[email protected]>
Signed-off-by: Hangbin Liu <[email protected]>
---
 drivers/net/bonding/bond_main.c | 34 ++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e45bba240cbc..683bf1221caf 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -537,6 +537,10 @@ static void bond_ipsec_add_sa_all(struct bonding *bond)
        }
 
        list_for_each_entry(ipsec, &bond->ipsec_list, list) {
+               /* Skip dead xfrm states, they'll be freed later. */
+               if (ipsec->xs->km.state == XFRM_STATE_DEAD)
+                       continue;
+
                /* If new state is added before ipsec_lock acquired */
                if (ipsec->xs->xso.real_dev == real_dev)
                        continue;
@@ -560,7 +564,6 @@ static void bond_ipsec_del_sa(struct xfrm_state *xs)
        struct net_device *bond_dev = xs->xso.dev;
        struct net_device *real_dev;
        netdevice_tracker tracker;
-       struct bond_ipsec *ipsec;
        struct bonding *bond;
        struct slave *slave;
 
@@ -592,15 +595,6 @@ static void bond_ipsec_del_sa(struct xfrm_state *xs)
        real_dev->xfrmdev_ops->xdo_dev_state_delete(xs);
 out:
        netdev_put(real_dev, &tracker);
-       mutex_lock(&bond->ipsec_lock);
-       list_for_each_entry(ipsec, &bond->ipsec_list, list) {
-               if (ipsec->xs == xs) {
-                       list_del(&ipsec->list);
-                       kfree(ipsec);
-                       break;
-               }
-       }
-       mutex_unlock(&bond->ipsec_lock);
 }
 
 static void bond_ipsec_del_sa_all(struct bonding *bond)
@@ -617,6 +611,12 @@ static void bond_ipsec_del_sa_all(struct bonding *bond)
 
        mutex_lock(&bond->ipsec_lock);
        list_for_each_entry(ipsec, &bond->ipsec_list, list) {
+               if (ipsec->xs->km.state == XFRM_STATE_DEAD) {
+                       list_del(&ipsec->list);
+                       kfree(ipsec);
+                       continue;
+               }
+
                if (!ipsec->xs->xso.real_dev)
                        continue;
 
@@ -640,6 +640,7 @@ static void bond_ipsec_free_sa(struct xfrm_state *xs)
        struct net_device *bond_dev = xs->xso.dev;
        struct net_device *real_dev;
        netdevice_tracker tracker;
+       struct bond_ipsec *ipsec;
        struct bonding *bond;
        struct slave *slave;
 
@@ -659,13 +660,24 @@ static void bond_ipsec_free_sa(struct xfrm_state *xs)
        if (!xs->xso.real_dev)
                goto out;
 
-       WARN_ON(xs->xso.real_dev != real_dev);
+       if (xs->xso.real_dev != real_dev)
+               goto out;
 
        if (real_dev && real_dev->xfrmdev_ops &&
            real_dev->xfrmdev_ops->xdo_dev_state_free)
                real_dev->xfrmdev_ops->xdo_dev_state_free(xs);
 out:
        netdev_put(real_dev, &tracker);
+
+       mutex_lock(&bond->ipsec_lock);
+       list_for_each_entry(ipsec, &bond->ipsec_list, list) {
+               if (ipsec->xs == xs) {
+                       list_del(&ipsec->list);
+                       kfree(ipsec);
+                       break;
+               }
+       }
+       mutex_unlock(&bond->ipsec_lock);
 }
 
 /**
-- 
2.46.0


Reply via email to