Re: [PATCH] mac802154: add a check for slave data list before delete

[Miquel Raynal]

On 12/11/2024 at 21:41:45 +08, Lizhi Xu <lizhi.xu@xxxxxxxxxxxxx> wrote:

> On Tue, 12 Nov 2024 12:01:21 +0100, Miquel Raynal wrote:
>>On 12/11/2024 at 08:21:33 +08, Lizhi Xu <lizhi.xu@xxxxxxxxxxxxx> wrote:
>>
>>> On Mon, 11 Nov 2024 20:46:57 +0100, Miquel Raynal wrote:
>>>> On 08/11/2024 at 22:54:20 +08, Lizhi Xu <lizhi.xu@xxxxxxxxxxxxx> wrote:
>>>>
>>>> > syzkaller reported a corrupted list in ieee802154_if_remove. [1]
>>>> >
>>>> > Remove an IEEE 802.15.4 network interface after unregister an IEEE 802.15.4
>>>> > hardware device from the system.
>>>> >
>>>> > CPU0					CPU1
>>>> > ====					====
>>>> > genl_family_rcv_msg_doit		ieee802154_unregister_hw
>>>> > ieee802154_del_iface			ieee802154_remove_interfaces
>>>> > rdev_del_virtual_intf_deprecated	list_del(&sdata->list)
>>>> > ieee802154_if_remove
>>>> > list_del_rcu
>>>>
>>>> FYI this is a "duplicate" but with a different approach than:
>>>> https://lore.kernel.org/linux-wpan/87v7wtpngj.fsf@xxxxxxxxxxx/T/#m02cebe86ec0171fc4d3350676bbdd4a7e3827077
>>> No, my patch was the first to fix it, someone else copied my
>>> patch. Here is my patch:
>>
>>Ok, so same question as to the other contributor, why not enclosing the
>>remaining list_del_rcu() within mutex protection? Can we avoid the
>>creation of the LISTDONE state bit?
> From the analysis of the list itself, we can not rely on the newly added state bit. 
> The net device has been unregistered, since the rcu grace period,
> unregistration must be run before ieee802154_if_remove.
>
> Following is my V2 patch, it has been tested and works well.

Please send a proper v2, not an inline v2.

However the new approach looks better to me, so you can add my

Reviewed-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>

Thanks,
Miquèl