Re: [PATCH net v4] nfc: ... device_is_registered() is data race-able

[Greg KH]

On Thu, Apr 28, 2022 at 09:53:28PM +0800, Lin Ma wrote:
> Hello there
> 
> > 
> > How do you physically remove a NFC device from a system?  What types of
> > busses are they on?  If non-removable one, then odds are there's not
> > going to be many races so this is a low-priority thing.  If they can be
> > on removable busses (USB, PCI, etc.), then that's a bigger thing.
> 
> Well, these are great questions we didn't even touch to yet.
> For the BT, HAMRADIO, and NFC/NCI code, we simply use pseudo-terminal + line
> discipline to emulate a malicious device from user-space (CAP_NET_ADMIN required).
> 
> We will then survey the actual physical bus for the IRL used NFC device.
> 
> > 
> > But again, the NFC bus code should handle all of this for the drivers,
> > there's nothing special about NFC that should warrant a special need for
> > this type of thing.
> > 
> 
> Agree, but in my opinion, every layer besides the bus code has to handle this race.
> 
> The cleanup routine is from
> "down" to "up" like ... -> TTY -> NFCMRVL -> NCI -> NFC core
> while the other routines, like netlink command is from "up" to "down"
> user space -> netlink -> NFC -> NCI -> NFCMRVL -> ...
> 
> Their directions are totally reversed hence only each layer of the stack perform good
> synchronization can the code be race free.
> 
> For example, this detaching event awake code in bus, the running task in higher layer
> is not aware of this. The cleanup of each layer has to make sure any running code won't
> cause use-after-free. 

That is what proper reference counting is supposed to be for.  Perhaps
you are running into a driver subsystem that is not doing that well, or
at all?

Try adding the needed references and the use-after-free should almost be
impossible to happen.

thanks,

greg k-h