[PATCH net] ieee802154: hwsim: free PIB after unregistering hardware

[PATCH net] ieee802154: hwsim: free PIB after unregistering hardware

[Yousef Alhouseen]

hwsim_del() queues the currently published PIB for RCU freeing before
unregistering the hardware. The unregister path can still invoke driver
callbacks, including set_promiscuous_mode(), after that grace period has
started. A callback can consequently dereference the freed PIB.

Unregister the hardware first, then fetch and free the final PIB. This also
handles a PIB replacement performed by a callback during unregister.

Fixes: 1c9f4a3fce77 ("ieee802154: hwsim: fix rcu handling")
Reported-by: syzbot+4707bb8a43a42fca2b97@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=4707bb8a43a42fca2b97
Cc: stable@vger.kernel.org
Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/net/ieee802154/mac802154_hwsim.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ieee802154/mac802154_hwsim.c b/drivers/net/ieee802154/mac802154_hwsim.c
index 6daa0f198..2a2d8a9eb 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -1004,12 +1004,11 @@ static void hwsim_del(struct hwsim_phy *phy)
 		list_del_rcu(&e->list);
 		hwsim_free_edge(e);
 	}
-	pib = rcu_dereference(phy->pib);
 	rcu_read_unlock();
 
-	kfree_rcu(pib, rcu);
-
 	ieee802154_unregister_hw(phy->hw);
+	pib = rcu_dereference_protected(phy->pib, 1);
+	kfree_rcu(pib, rcu);
 	ieee802154_free_hw(phy->hw);
 }
 
-- 
2.54.0

Re: [PATCH net] ieee802154: hwsim: free PIB after unregistering hardware

[Miquel Raynal]

Hello Yousef,

> @@ -1004,12 +1004,11 @@ static void hwsim_del(struct hwsim_phy *phy)
>  		list_del_rcu(&e->list);
>  		hwsim_free_edge(e);
>  	}
> -	pib = rcu_dereference(phy->pib);
>  	rcu_read_unlock();
>  
> -	kfree_rcu(pib, rcu);
> -
>  	ieee802154_unregister_hw(phy->hw);
> +	pib = rcu_dereference_protected(phy->pib, 1);
> +	kfree_rcu(pib, rcu);
>  	ieee802154_free_hw(phy->hw);
>  }

Would you mind justifying the choice for the _protected() version,
please?

Thanks,
Miquèl

Re: [PATCH net] ieee802154: hwsim: free PIB after unregistering hardware

[Yousef Alhouseen]

Hello Miquel,

After ieee802154_unregister_hw() returns, the driver callbacks that
can replace phy->pib have been quiesced, and hwsim_del() has exclusive
ownership of the final teardown. The pointer is no longer being
fetched inside an RCU read-side critical section, so rcu_dereference()
is not appropriate there.

rcu_dereference_protected(phy->pib, 1) expresses that there can no
longer be a concurrent updater at that point; the protection condition
is the completed unregister rather than a locally held lock. The value
is only fetched so the final object can be passed to kfree_rcu().

rcu_access_pointer() would also be sufficient for that limited use if
you prefer it, and I can use that spelling in a v2.

Thanks,
Yousef

On Fri, 03 Jul 2026 13:15:46 +0200, Miquel Raynal
<miquel.raynal@bootlin.com> wrote:
> Hello Yousef,
>
> > @@ -1004,12 +1004,11 @@ static void hwsim_del(struct hwsim_phy *phy)
> > list_del_rcu(&e->list);
> > hwsim_free_edge(e);
> > }
> > - pib = rcu_dereference(phy->pib);
> > rcu_read_unlock();
> >
> > - kfree_rcu(pib, rcu);
> > -
> > ieee802154_unregister_hw(phy->hw);
> > + pib = rcu_dereference_protected(phy->pib, 1);
> > + kfree_rcu(pib, rcu);
> > ieee802154_free_hw(phy->hw);
> > }
>
> Would you mind justifying the choice for the _protected() version,
> please?
>
> Thanks,
> Miquèl

Re: [PATCH net] ieee802154: hwsim: free PIB after unregistering hardware

[Miquel Raynal]

Hello Yousef,

On 03/07/2026 at 04:19:42 -07, Yousef Alhouseen <alhouseenyousef@gmail.com> wrote:

> Hello Miquel,
>
> After ieee802154_unregister_hw() returns, the driver callbacks that
> can replace phy->pib have been quiesced, and hwsim_del() has exclusive
> ownership of the final teardown. The pointer is no longer being
> fetched inside an RCU read-side critical section, so rcu_dereference()
> is not appropriate there.

That's right.

> rcu_dereference_protected(phy->pib, 1) expresses that there can no
> longer be a concurrent updater at that point; the protection condition
> is the completed unregister rather than a locally held lock. The value
> is only fetched so the final object can be passed to kfree_rcu().

While I believe this is indeed true, it actually feels slightly overkill
since there is no updater anymore and, as far as I understand, the only
thing that we need here is to get the pointer value for freeing, right?

> rcu_access_pointer() would also be sufficient for that limited use if
> you prefer it, and I can use that spelling in a v2.

If you don't mind, I feel like rcu_access_pointer() is exactly what we
need here. It is also more understandable, even though I agree in
practice there are no differences in this case.

Thanks,
Miquèl