lkml.org 
[lkml]   [2022]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH net v4] nfc: ... device_is_registered() is data race-able
Hello Greg,


>
> You should not be making these types of checks outside of the driver
> core.
>
> > This is by no means matching our expectations as one of our previous patch relies on the device_is_registered code.
>
> Please do not do that.
>
> >
> > -> the patch: 3e3b5dfcd16a ("NFC: reorder the logic in nfc_{un,}register_device")
> >
> <...>
> >
> > In another word, the device_del -> kobject_del -> __kobject_del is not protected by the device_lock.
>
> Nor should it be.
>

I may have mistakenly presented my point. In fact, there is nothing wrong with the device core, nothing to do with the internal of device_del and device_is_registered implementation. And, of course, we will not add any code or do any modification to the device/driver base code.

The point is the combination of device_is_registered + device_del, which is used in NFC core, is not safe.

That is to say, even the device_is_registered can return True even the device_del is executing in another thread.

(By debugging we think this is true, correct me if it is not)

Hence we want to add additional state in nfc_dev object to fix that, not going to add any state in device/driver core.

> > This means the device_lock + device_is_registered is still prone to the data race. And this is not just the problem with firmware downloading. The all relevant netlink tasks that use the device_lock + device_is_registered is possible to be raced.
> >
> > To this end, we will come out with two patches, one for fixing this device_is_registered by using another status variable instead. The other is the patch that reorders the code in nci_unregister_device.
>
> Why is this somehow unique to these devices? Why do no other buses have
> this issue? Are you somehow allowing a code path that should not be
> happening?
>
> thanks,
>
> greg k-h

In fact, by searching the device_is_registered() use cases, I found that most of them are used in drier code instead of in the network stack. I have no idea whether or not they suffer from similar problems and I will check that out.

Thanks
Lin
\
 
 \ /
  Last update: 2022-04-28 09:55    [W:0.105 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site