hwsim_update_pib() does an unserialized read-swap-free of phy->pib:
pib_old = rtnl_dereference(phy->pib);
...
rcu_assign_pointer(phy->pib, pib);
kfree_rcu(pib_old, rcu);
It assumes the RTNL is held, but ->set_channel is not always called
under it: the mac802154 scan worker changes channels via
drv_set_channel() without the RTNL. Such an update can race an
RTNL-held one on the same phy; both read the same pib_old and both
kfree_rcu() it, double-freeing the object. With SLUB percpu sheaves
batching kfree_rcu(), this surfaces as a KASAN invalid-free in
rcu_free_sheaf().
struct hwsim_phy has no lock for pib. Add one and make the swap atomic
with rcu_replace_pointer() under it, dropping the misleading
rtnl_dereference().
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=60332fd095f8bb2946ad
Fixes: f25da51fdc38 ("ieee802154: hwsim: add replacement for fakelb")
Signed-off-by: David Carlier <[email protected]>
Cc: <[email protected]>
---
drivers/net/ieee802154/mac802154_hwsim.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c
b/drivers/net/ieee802154/mac802154_hwsim.c
index 6daa0f198b9f..a9bd1555d2dc 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -72,6 +72,8 @@ struct hwsim_phy {
struct ieee802154_hw *hw;
u32 idx;
+ /* Serializes phy->pib_updates. */
+ spinlock_t pib_lock;
struct hwsim_pib __rcu *pib;
bool suspended;
@@ -102,8 +104,6 @@ static int hwsim_update_pib(struct ieee802154_hw *hw, u8
page, u8 channel,
if (!pib)
return -ENOMEM;
- pib_old = rtnl_dereference(phy->pib);
-
pib->page = page;
pib->channel = channel;
pib->filt.short_addr = filt->short_addr;
@@ -112,7 +112,10 @@ static int hwsim_update_pib(struct ieee802154_hw *hw, u8
page, u8 channel,
pib->filt.pan_coord = filt->pan_coord;
pib->filt_level = filt_level;
- rcu_assign_pointer(phy->pib, pib);
+ spin_lock_bh(&phy->pib_lock);
+ pib_old = rcu_replace_pointer(phy->pib, pib,
+ lockdep_is_held(&phy->pib_lock));
+ spin_unlock_bh(&phy->pib_lock);
kfree_rcu(pib_old, rcu);
return 0;
}
@@ -952,6 +955,7 @@ static int hwsim_add_one(struct genl_info *info, struct
device *dev,
goto err_pib;
}
+ spin_lock_init(&phy->pib_lock);
pib->channel = 13;
pib->filt.short_addr = cpu_to_le16(IEEE802154_ADDR_BROADCAST);
pib->filt.pan_id = cpu_to_le16(IEEE802154_PANID_BROADCAST);
--
2.53.0
