Re: [PATCH] net: nfc: fix deadlock between nfc_unregister_device and rfkill_fop_write

[Deepanshu Kartikey]

On Wed, Dec 17, 2025 at 1:01 PM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote:
>
> 1. So your code allows concurrent thread nfc_rfkill_set_block() to be
> called at this spot
> 2. Original thread of unregistering will shortly later call
> device_del(), which goes through lock+kill+unlock,
> 3. Then the concurrent thread proceeds to device_lock() and all other
> things with freed device.
>
> You just replaced one issue with another issue, right?
>

Hi Krzysztof,

Thanks for the review.

Regarding the UAF concern:

The callback nfc_rfkill_set_block() is invoked from rfkill_fop_write()
which holds rfkill_global_mutex for the entire operation:

rfkill_fop_write():
    mutex_lock(&rfkill_global_mutex);
    list_for_each_entry(rfkill, &rfkill_list, node) {
        rfkill_set_block(rfkill, ev.soft);
    }
    mutex_unlock(&rfkill_global_mutex);

rfkill_set_block() calls ops->set_block() (i.e., nfc_rfkill_set_block)
without releasing rfkill_global_mutex.

Since rfkill_unregister() also acquires rfkill_global_mutex:

void rfkill_unregister(struct rfkill *rfkill)
{
    ...
    mutex_lock(&rfkill_global_mutex);
    rfkill_send_events(rfkill, RFKILL_OP_DEL);
    list_del_init(&rfkill->node);
    ...
    mutex_unlock(&rfkill_global_mutex);
}

The unregister path cannot proceed past rfkill_unregister() until any
ongoing callback completes. Since device_del() is called after
rfkill_unregister() returns, no UAF should be possible.

Additionally, if nfc_dev_down() runs after we set shutting_down = true,
it will see the flag and bail out early with -ENODEV without accessing
device internals.

Regarding nfc_register_device(): The same lock ordering exists there
(device_lock -> rfkill_global_mutex via rfkill_register), but during
registration the device is not yet visible to other subsystems, so no
concurrent rfkill operations can occur. The ABBA pattern there should
not cause an actual deadlock.

I will send a v2 addressing:
- Adding Fixes and Cc: stable tags
- Keeping the blank line after variable declaration

Thanks,
Deepanshu