Re: [PATCH] bdi: fix use-after-free for bdi device

[Yufen Yu <yuyufen@huawei.com>]

On 2020/2/13 21:58, Tejun Heo wrote:
> Hello,
> 
> On Thu, Feb 13, 2020 at 03:51:40PM +0800, Yufen Yu wrote:
>> If we destroy the device asynchronously by call_rcu(), we may need to
>> add a new member 'rcu_head' into struct backing_dev_info. Right?
> 
> Yes.
> 
>> The code may be like:
>>
>> bdi_unregister()
>> {
>> 	...
>> 	if (bdi->dev) {
>> 		...
>> 		device_get(bdi->dev);
>> 		device_unregister(bdi->dev);
>> 		bdi->dev = NULL; //XXX
>> 		bdi_get(bdi); //avoiding bdi to be freed before calling bdi_release_device
>> 		call_rcu(&bdi->rcu_head, bdi_release_device);
>> 	}
>> 		...
>> }
>>
>> bdi_release_device()
>> {
>> 	...
>> 	put_device(bdi->dev);//XXX
>> 	bdi_put(bdi);
>> }
>>
>> But, the problem is how do we get 'bdi->dev' in bdi_release_device().
>> If we do not set bdi->dev as 'NULL', re-registration bdi may cannot work well.
> 
> So, unregistering can leave ->dev along and re-registering can test
> whether it's NULL and if not put the existing one and put a new one
> there. Wouldn't that work?

Do you mean set bdi->dev as 'NULL' in call_rcu() callback function
(i.e. bdi_release_device()) and test 'bdi->dev' in bdi_register_va()?

I think that may do not work.
We cannot make sure the order of rcu callback function and re-registering.
Then bdi_release_device() may put the new allocated device by re-registering.

Thanks,
Yufen