Re: [PATCH 1/2] dmaengine: idxd: Remove improper idxd_free

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Yi Sun <yi.sun@intel.com> writes:

> On 29.05.2025 09:56, Vinicius Costa Gomes wrote:
>>Hi,
>>
>>Yi Sun <yi.sun@intel.com> writes:
>>
>>> The put_device() call can be asynchronous cleanup via schedule_delayed_work
>>> when CONFIG_DEBUG_KOBJECT_RELEASE is set. This results in a use-after-free
>>> failure during module unloading if invoking idxd_free() immediately
>>> afterward.
>>>
>>
>>I think that adding the relevant part of the log would be helpful. (I am
>>looking at either a similar, or this exact problem, so at least to me it
>>would be helpful)
>>
> The issue is easily reproducible: unloading the module with 'modprobe -r idxd'
> can trigger the call trace so long as a idxd_free() is called immediately
> after the put_device().
>

Most probably the same issue I am looking at then. 

> I can include the call trace in the next version commit log if it's helpful.
>

Yes, that's helpful. Usually it's better to include more information in
the commit message.

> [ 1957.463315] refcount_t: underflow; use-after-free.
> [ 1957.463337] WARNING: CPU: 15 PID: 4428 at lib/refcount.c:28 refcount_warn_saturate+0xbe/0x110
> ... ...
> [ 1957.463424] RIP: 0010:refcount_warn_saturate+0xbe/0x110
> ... ...
> [ 1957.463445] Call Trace:
> [ 1957.463450]  <TASK>
> [ 1957.463458]  idxd_remove+0xe4/0x120 [idxd]
> [ 1957.463497]  pci_device_remove+0x3f/0xb0
> [ 1957.463505]  device_release_driver_internal+0x197/0x200
> [ 1957.463513]  driver_detach+0x48/0x90
> [ 1957.463515]  bus_remove_driver+0x74/0xf0
> [ 1957.463521]  pci_unregister_driver+0x2e/0xb0
> [ 1957.463524]  idxd_exit_module+0x34/0x7a0 [idxd]
> [ 1957.463529]  __do_sys_delete_module.constprop.0+0x183/0x280
> [ 1957.463536]  ? syscall_trace_enter+0x163/0x1c0
> [ 1957.463540]  do_syscall_64+0x54/0xd70
> [ 1957.463549]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 1957.463555] RIP: 0033:0x7fb52b10ee2b
>
>>> Removes the improper call idxd_free() to prevent potential memory
>>> corruption.
>>
>>Thinking if it would be worth a Fixes: tag.
>>
> Yes, sure. Will add Fixes: tag next version.
>
> Thanks
>     --Sun, Yi


Thank you,
-- 
Vinicius