netdev - Re: [PATCH net-next 1/2] mac80211_hwsim: Make hwsim

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5319c18f-ce39-0d4c-e140-50b48b73803f@virtuozzo.com>
Date:   Fri, 2 Mar 2018 14:37:25 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     Benjamin Beichler <Benjamin.Beichler@...-rostock.de>,
        davem@...emloft.net, johannes@...solutions.net,
        kvalo@...eaurora.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] mac80211_hwsim: Make hwsim_netgroup IDA

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