Re: [PATCH net-next 1/2] mac80211_hwsim: Make hwsim_netgroup IDA

[Kirill Tkhai <ktkhai@virtuozzo.com>]

On 01.03.2018 20:22, Benjamin Beichler wrote:
> Am 01.03.2018 um 12:30 schrieb Kirill Tkhai:
> 
>> Out of bounds of this patch, just as a report to wireless subsystem
>> maintainer, destroy_radio() increaments hwsim_radios_generation
>> without hwsim_radio_lock, so this may need one more patch to fix.
>>
> The lock is here implicit, because the value only could change at init
> (where netlink callbacks are deactivated and always happens sequential)
> or in netlink callbacks. The only reader of this variable is the dump
> callback, and the only other writers are also netlink callbacks and
> because they are implicitly not parallel (because the parallel flag is
> not set), there could be no race condition. Maybe this should be
> documented somehow, especially if somebody got the idea to allow
> parallel callbacks :-)


static void hwsim_exit_net(...)
{
	...
	INIT_WORK(&data->destroy_work, destroy_radio);
        queue_work(hwsim_wq, &data->destroy_work);
	...
}

destroy_radio() may be executed in parallel with everything above you wrote,
doesn't it? There may be several network namespaces, and destroy_radio()
queued from one net namespace may race with mac80211_hwsim_new_radio()
or hwsim_del_radio_nl() for another net namespace. I don't see, how netlink
locking can act on synchronization with a work. This is what I mention.

Thanks,
Kirill