linux-kernel - Re: [PATCH 2/2] dmaengine: idxd: Fix refcount underflow on module unload

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: <aDkgxCsCtsEugTdI@ysun46-mobl.ccr.corp.intel.com>
Date: Fri, 30 May 2025 11:06:44 +0800
From: Yi Sun <yi.sun@...el.com>
To: Vinicius Costa Gomes <vinicius.gomes@...el.com>
CC: <dave.jiang@...el.com>, <dmaengine@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <xueshuai@...ux.alibaba.com>,
	<gordon.jin@...el.com>
Subject: Re: [PATCH 2/2] dmaengine: idxd: Fix refcount underflow on module
 unload

On 29.05.2025 10:04, Vinicius Costa Gomes wrote:
>Yi Sun <yi.sun@...el.com> writes:
>
>> A recent refactor introduced a misplaced put_device() call, resulting in
>> reference count underflow when the module is unloaded.
>>
>> Expand the idxd_cleanup() function to handle proper cleanup, and remove
>> idxd_cleanup_internals() as it was not part of the driver unload path.
>>
>
>'idxd_cleanup_internals()' frees a bunch of stuff. I would expect an
>explanation of when those things are being free'd now that removed that
>call.
>

I believe the call to idxd_unregister_devices(), which is invoked at the
very beginning of idxd_remove(), already takes care of the necessary
put_device() through the following call path:
idxd_unregister_devices() -> device_unregister() -> put_device()

Therefore, there's no need to add additional put_device() calls for idxd
groups, engines, or workqueues. While the commit message for a409e919ca3
states: "Note, this also fixes the missing put_device() for idxd groups,
engines, and wqs."
it appears that no such omission actually existed, this part of the flow
was already correctly handled.

Moreover, this refcount underflow issue appears to be a a clear
regression. Prior to this commit, idxd_cleanup_internals() was not part of
the driver unload path. The commit did not provide a strong justification
for calling idxd_cleanup_internals() within idxd_cleanup().

For reference, the both two related bugs produce nearly identical call
traces, and I think both are blocking issues.

Thanks
    --Sun, Yi