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

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: <YmqgxNkXVetmrtde@kroah.com>
Date:   Thu, 28 Apr 2022 16:12:20 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Lin Ma <linma@....edu.cn>
Cc:     Duoming Zhou <duoming@....edu.cn>, krzysztof.kozlowski@...aro.org,
        pabeni@...hat.com, linux-kernel@...r.kernel.org,
        davem@...emloft.net, alexander.deucher@....com,
        akpm@...ux-foundation.org, broonie@...nel.org,
        netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net v4] nfc: ... device_is_registered() is data race-able

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