Re: [PATCH] PCI: endpoint: Fix configfs group removal on driver teardown

[Niklas Cassel <cassel@kernel.org>]

Hello Damien,

On Tue, Jun 17, 2025 at 10:07:37AM +0900, Damien Le Moal wrote:
> An endpoint driver configfs attributes group is added to the
> epf_group list of struct pci_epf_driver pci_epf_add_cfs() but not
> removed from this list when the attribute group is unregistered with
> pci_ep_cfs_remove_epf_group(). Add the missing list_del_init() call in
> that function to correctly remove the attribute group from the driver
> list.

This seems like a bug (bug #1).

> 
> Furthermore, doing a list_del() on the epf_group field of struct
> pci_epf_driver in pci_epf_remove_cfs() is not correct as this field is a
> list head, not a list entry, and triggers a KASAN warning:

This seems like another bug (bug #2).


I do understand that both bugs were introduced by the same commit.

However, since it is two separate bugs, I personally think that they
deserve two separate patches (even if they will have the same Fixes tag).


> 
> ==================================================================
> BUG: KASAN: slab-use-after-free in pci_epf_remove_cfs+0x17c/0x198
> Write of size 8 at addr ffff00010f4a0d80 by task rmmod/319
> 
> CPU: 3 UID: 0 PID: 319 Comm: rmmod Not tainted 6.16.0-rc2 #1 NONE
> Hardware name: Radxa ROCK 5B (DT)
> Call trace:
> show_stack+0x2c/0x84 (C)
> dump_stack_lvl+0x70/0x98
> print_report+0x17c/0x538
> kasan_report+0xb8/0x190
> __asan_report_store8_noabort+0x20/0x2c
> pci_epf_remove_cfs+0x17c/0x198
> pci_epf_unregister_driver+0x18/0x30
> nvmet_pci_epf_cleanup_module+0x24/0x30 [nvmet_pci_epf]
> __arm64_sys_delete_module+0x264/0x424
> invoke_syscall+0x70/0x260
> el0_svc_common.constprop.0+0xac/0x230
> do_el0_svc+0x40/0x58
> el0_svc+0x48/0xdc
> el0t_64_sync_handler+0x10c/0x138
> el0t_64_sync+0x198/0x19c

This KASAN splat seems to belong to bug #2.



I think it is a litte bit confusing that you attach a KASAN
splat to a patch that fixes two different bugs.

Surely this KASAN bug can be fixes with only:

-     list_del(&driver->epf_group);
+     WARN_ON(!list_empty(&driver->epf_group));


So I think it would make more sense if the patch that fixes the KASAN splat
includes only the changes that are needed to fix the KASAN splat, and then
for the other bug, create a different patch that will then have a clearer
commit message (because it will not have an unrelated KASAN splat in it).


Kind regards,
Niklas