Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store

[Pei Xiao <xiaopei01@kylinos.cn>]

在 2026/8/31 17:32, Niklas Cassel 写道:
> Hello Pei,
> 
> On Mon, Aug 31, 2026 at 11:26:06AM +0200, Niklas Cassel wrote:
>> On Thu, 30 Jul 2026 11:16:29 +0800, Pei Xiao wrote:
>>> The function new_device_store() calls driver_find() without any
>>> protection against concurrent driver unregistration. This can lead
>>> to a use-after-free (UAF) when a driver is unloaded (via rmmod)
>>> in parallel with a new device addition via sysfs.
>>>
>>> The race window exists because driver_find() returns a pointer to
>>> the driver's private data, but does not increase its reference
>>> count. The caller is responsible for ensuring the driver remains
>>> valid, but new_device_store() did not hold any lock or reference
>>> during the lookup and subsequent use.
>>>
>>> [...]
>>
>> Applied to libata/linux.git (for-7.4), thanks!
>>
>> [1/1] ata: pata_parport: Fix use-after-free in new_device_store
>>       https://git.kernel.org/libata/linux/c/bd46a0b2
> 
> I picked up this patch.
> 
> But here:
> https://lore.kernel.org/linux-ide/dd146e49-33ff-4ae8-a641-1dc614e22733@kylinos.cn/T/#m1cd93326f2935273e707c0aaea3c659a238bf2b6
> 
> Damien asked you:
> "Sashiko had a comment about this that I think is very valid: if rmmod is
> executed with devices attached, what happens here?
> This entire driver seems to be lacking reference counting on the
> modules/drivers, so this all seems very fragile."
hi Maintainer,
I have submitted this patch, and now I'm a bit unsure whether I've
solved this problem.
https://lore.kernel.org/lkml/a8d9807618ab786313654099c904d613075e1b63.1785398816.git.xiaopei01@kylinos.cn/

thanks！
Pei.
> 
> 
> The Sashiko comment he was referring to was not a Sashiko comment posted in
> that same thread, but on an earlier version of your patch. The Sashiko
> comment can be found here:
> https://lore.kernel.org/linux-ide/20260729112609.3CD3A1F000E9@smtp.kernel.org/
> 
> """"
> [Severity: High]
> This is a pre-existing issue, but does pata_parport_unregister_driver() leak
> devices?
> 
> When a protocol module's init function registers multiple protocols (like
> kbic_init registering k951 and k971) and a subsequent registration fails, it
> will call pata_parport_unregister_driver() on the already-registered protocol.
> 
> While the protocol is removed from the IDR and the driver is unregistered, the
> dynamically created pi_adapter devices are not cleaned up. Since the module
> init returns an error, the module loader frees the module memory, bypassing
> the reference held by the devices.
> 
> If these dangling devices are later removed (for example, via sysfs
> delete_device), pi_remove_one() calls pi_disconnect(pi), which dereferences
> the freed pi->proto->disconnect pointer, leading to a kernel crash.
> 
> Should the associated devices be unregistered here as well?
> """"
> 
> 
> Do you perhaps have some spare cycles to address this issue as well?
> 
> 
> Kind regards,
> Niklas