* [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
@ 2026-07-30 3:16 Pei Xiao
2026-07-30 3:29 ` sashiko-bot
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Pei Xiao @ 2026-07-30 3:16 UTC (permalink / raw)
To: dlemoal, cassel, linux, linux-ide, linux-kernel; +Cc: Pei Xiao, Shuangpeng Bai
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.
Concurrently, pata_parport_unregister_driver() releases the
pi_mutex before calling driver_unregister(), allowing new_device_store
to proceed with a stale pointer after the driver has been freed.
Fix this by expanding the critical section protected by pi_mutex
in both functions:
- In new_device_store(): acquire pi_mutex before calling driver_find(),
and keep it held until all uses of the found driver pointer are
completed.
- In pata_parport_unregister_driver(): hold pi_mutex throughout the
entire unregistration process, including the driver_unregister()
call, so that no concurrent lookup can see a partially removed
driver.
Thread A (new_device_store) | Thread B (pata_parport_unregister_driver)
driver_find("ate") |
| mutex_lock(&pi_mutex)
| idr_remove(&protocols, id)
| mutex_unlock(&pi_mutex)
| driver_unregister(&pr->driver)
| (frees driver_private)
/* continues using stale driv */|
-> UAF! |
Logs:
BUG: KASAN: slab-use-after-free in driver_find+0xd0/0xd4
Read of size 8 at addr ffffff9f970dc690 by task sh/4737
Call trace:
show_stack+0x14/0x1c (C)
dump_stack_lvl+0x70/0x84
print_report+0xf4/0x5a4
kasan_report+0xa0/0xe4
__asan_report_load8_noabort+0x18/0x20
driver_find+0xd0/0xd4
new_device_store+0x140/0x30c [pata_parport]
bus_attr_store+0x5c/0x94
sysfs_kf_write+0x1c4/0x25c
...
Allocated by task 4734:
kasan_save_stack+0x28/0x4c
kasan_save_track+0x1c/0x34
kasan_save_alloc_info+0x3c/0x4c
__kasan_kmalloc+0x98/0xac
__kmalloc_cache_noprof+0x158/0x3cc
bus_add_driver+0x70/0x4d8
driver_register+0xf0/0x3b0
pata_parport_register_driver+0xb8/0x1c4 [pata_parport]
0xffffffc07f5ce014
do_one_initcall+0xb8/0x36c
do_init_module+0x230/0x6d4
...
Freed by task 4737:
kasan_save_stack+0x28/0x4c
kasan_save_track+0x1c/0x34
kasan_save_free_info+0x48/0x8c
__kasan_slab_free+0x5c/0x84
kfree+0x174/0x3e0
driver_release+0x1c/0x7c
kobject_put+0x178/0x47c
driver_find+0x78/0xd4
new_device_store+0x140/0x30c [pata_parport]
bus_attr_store+0x5c/0x94
sysfs_kf_write+0x1c4/0x25c
...
Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
Closes: https://lore.kernel.org/lkml/20260728024015.2014674-1-shuangpeng.kernel@gmail.com/
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v2:
1.Modify to use the correct fix ID.
2.Use mutex_lock instead of guard.
---
drivers/ata/pata_parport/pata_parport.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
index 40baeac594a9..cf81a6128f55 100644
--- a/drivers/ata/pata_parport/pata_parport.c
+++ b/drivers/ata/pata_parport/pata_parport.c
@@ -618,8 +618,9 @@ void pata_parport_unregister_driver(struct pi_protocol *pr)
break;
}
idr_remove(&protocols, id);
- mutex_unlock(&pi_mutex);
driver_unregister(&pr->driver);
+ mutex_unlock(&pi_mutex);
+
}
EXPORT_SYMBOL_GPL(pata_parport_unregister_driver);
@@ -646,18 +647,18 @@ static ssize_t new_device_store(const struct bus_type *bus, const char *buf, siz
port_wanted = -1;
}
+ mutex_lock(&pi_mutex);
drv = driver_find(protocol, &pata_parport_bus_type);
if (!drv) {
if (strcmp(protocol, "auto")) {
pr_err("protocol %s not found\n", protocol);
+ mutex_unlock(&pi_mutex);
return -EINVAL;
}
pr_wanted = NULL;
} else {
pr_wanted = container_of(drv, struct pi_protocol, driver);
}
-
- mutex_lock(&pi_mutex);
/* walk all parports */
idr_for_each_entry(&parport_list, parport, port_num) {
if (port_num == port_wanted || port_wanted == -1) {
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-07-30 3:16 [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store Pei Xiao
@ 2026-07-30 3:29 ` sashiko-bot
2026-08-10 2:26 ` Pei Xiao
2026-08-31 9:26 ` Niklas Cassel
2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-07-30 3:29 UTC (permalink / raw)
To: Pei Xiao; +Cc: cassel, linux-ide, dlemoal
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Iterating over multiple protocols without breaking after a successful probe causes a permanent deadlock on `parport_claim_or_block()`.
- [High] The `new_device` sysfs attribute does not enforce positive bounds for `mode`, `unit`, and `delay`, allowing negative values that cause out-of-bounds array accesses and CPU stalls.
--
--- Patch [1]: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store ---
commit 0903251c06a3e624fab8c81fa244b1596978b12b
Author: Pei Xiao <xiaopei01@kylinos.cn>
ata: pata_parport: Fix use-after-free in new_device_store
The function new_device_store() calls driver_find() without protection against
concurrent driver unregistration. This commit expands the critical section
protected by pi_mutex to fix a use-after-free.
Link: https://lore.kernel.org/lkml/20260728024015.2014674-1-shuangpeng.kernel@gmail.com/
> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
[ ... ]
> @@ -646,18 +647,18 @@ static ssize_t new_device_store(const struct bus_type *bus, const char *buf, siz
> port_wanted = -1;
> }
>
[Severity: High]
This is a pre-existing issue, but does this function properly validate the
bounds of user input for mode, unit, and delay?
If negative integers are provided in the sysfs write to new_device, they are
parsed as signed integers:
if (sscanf(buf, "%11s %7s %d %d %d",
port, protocol, &mode, &unit, &delay) < 1)
These negative values can bypass upper-bound checks and be used as array
indices:
drivers/ata/pata_parport/bpck6.c:bpck6_send_cmd() {
...
switch (mode_map[pi->mode]) {
...
}
A negative delay can also be cast to a large unsigned value in udelay(),
potentially causing a permanent CPU stall:
drivers/ata/pata_parport/pata_parport.h:delay_p() {
#define delay_p (pi->delay ? udelay(pi->delay) : (void)0)
}
Could we enforce positive bounds for these parameters?
> + mutex_lock(&pi_mutex);
> drv = driver_find(protocol, &pata_parport_bus_type);
> if (!drv) {
> if (strcmp(protocol, "auto")) {
> pr_err("protocol %s not found\n", protocol);
> + mutex_unlock(&pi_mutex);
> return -EINVAL;
> }
> pr_wanted = NULL;
> } else {
> pr_wanted = container_of(drv, struct pi_protocol, driver);
> }
> -
> - mutex_lock(&pi_mutex);
> /* walk all parports */
> idr_for_each_entry(&parport_list, parport, port_num) {
> if (port_num == port_wanted || port_wanted == -1) {
[Severity: High]
This isn't a bug introduced by this patch, but does the protocol iteration
loop handle successful probes safely?
In the subsequent code in new_device_store():
/* walk all protocols */
idr_for_each_entry(&protocols, pr, pr_num) {
if (pr == pr_wanted || !pr_wanted)
if (pi_init_one(parport, pr, mode, unit,
delay))
ok = true;
}
And similarly in pata_parport_attach():
/* probe this port using all protocols */
idr_for_each_entry(&protocols, pr, pr_num)
pi_init_one(port, pr, -1, -1, -1);
If a protocol successfully initializes, it claims the parport exclusively.
However, the loop doesn't break upon success and continues trying the next
protocol.
When the next protocol calls pi_init_one() and attempts to claim the exact
same port, it will block indefinitely in parport_claim_or_block() because
the port is already locked by the first protocol.
Could this result in a permanent deadlock during sysfs-driven instantiation
or module loading?
--
Sashiko AI review · https://sashiko.dev/#/patchset/77dad1184ebd0e1e7ea541d4207568f8bc5ec01f.1785381141.git.xiaopei01@kylinos.cn?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-07-30 3:16 [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store Pei Xiao
2026-07-30 3:29 ` sashiko-bot
@ 2026-08-10 2:26 ` Pei Xiao
2026-08-31 9:26 ` Niklas Cassel
2 siblings, 0 replies; 8+ messages in thread
From: Pei Xiao @ 2026-08-10 2:26 UTC (permalink / raw)
To: dlemoal, cassel, linux, linux-ide, linux-kernel
在 2026/7/30 11:16, Pei Xiao 写道:
> 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.
>
> Concurrently, pata_parport_unregister_driver() releases the
> pi_mutex before calling driver_unregister(), allowing new_device_store
> to proceed with a stale pointer after the driver has been freed.
>
> Fix this by expanding the critical section protected by pi_mutex
> in both functions:
>
> - In new_device_store(): acquire pi_mutex before calling driver_find(),
> and keep it held until all uses of the found driver pointer are
> completed.
> - In pata_parport_unregister_driver(): hold pi_mutex throughout the
> entire unregistration process, including the driver_unregister()
> call, so that no concurrent lookup can see a partially removed
> driver.
>
> Thread A (new_device_store) | Thread B (pata_parport_unregister_driver)
> driver_find("ate") |
> | mutex_lock(&pi_mutex)
> | idr_remove(&protocols, id)
> | mutex_unlock(&pi_mutex)
> | driver_unregister(&pr->driver)
> | (frees driver_private)
> /* continues using stale driv */|
> -> UAF! |
>
> Logs:
> BUG: KASAN: slab-use-after-free in driver_find+0xd0/0xd4
> Read of size 8 at addr ffffff9f970dc690 by task sh/4737
> Call trace:
> show_stack+0x14/0x1c (C)
> dump_stack_lvl+0x70/0x84
> print_report+0xf4/0x5a4
> kasan_report+0xa0/0xe4
> __asan_report_load8_noabort+0x18/0x20
> driver_find+0xd0/0xd4
> new_device_store+0x140/0x30c [pata_parport]
> bus_attr_store+0x5c/0x94
> sysfs_kf_write+0x1c4/0x25c
> ...
>
> Allocated by task 4734:
> kasan_save_stack+0x28/0x4c
> kasan_save_track+0x1c/0x34
> kasan_save_alloc_info+0x3c/0x4c
> __kasan_kmalloc+0x98/0xac
> __kmalloc_cache_noprof+0x158/0x3cc
> bus_add_driver+0x70/0x4d8
> driver_register+0xf0/0x3b0
> pata_parport_register_driver+0xb8/0x1c4 [pata_parport]
> 0xffffffc07f5ce014
> do_one_initcall+0xb8/0x36c
> do_init_module+0x230/0x6d4
> ...
>
> Freed by task 4737:
> kasan_save_stack+0x28/0x4c
> kasan_save_track+0x1c/0x34
> kasan_save_free_info+0x48/0x8c
> __kasan_slab_free+0x5c/0x84
> kfree+0x174/0x3e0
> driver_release+0x1c/0x7c
> kobject_put+0x178/0x47c
> driver_find+0x78/0xd4
> new_device_store+0x140/0x30c [pata_parport]
> bus_attr_store+0x5c/0x94
> sysfs_kf_write+0x1c4/0x25c
> ...
>
> Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
Hi Cassel,
I have already the fix tag.
thanks!
Pei.
> Reported-by: Shuangpeng Bai <shuangpeng.kernel@gmail.com>
> Closes: https://lore.kernel.org/lkml/20260728024015.2014674-1-shuangpeng.kernel@gmail.com/
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
> changes in v2:
> 1.Modify to use the correct fix ID.
> 2.Use mutex_lock instead of guard.
> ---
> drivers/ata/pata_parport/pata_parport.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> index 40baeac594a9..cf81a6128f55 100644
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
> @@ -618,8 +618,9 @@ void pata_parport_unregister_driver(struct pi_protocol *pr)
> break;
> }
> idr_remove(&protocols, id);
> - mutex_unlock(&pi_mutex);
> driver_unregister(&pr->driver);
> + mutex_unlock(&pi_mutex);
> +
> }
> EXPORT_SYMBOL_GPL(pata_parport_unregister_driver);
>
> @@ -646,18 +647,18 @@ static ssize_t new_device_store(const struct bus_type *bus, const char *buf, siz
> port_wanted = -1;
> }
>
> + mutex_lock(&pi_mutex);
> drv = driver_find(protocol, &pata_parport_bus_type);
> if (!drv) {
> if (strcmp(protocol, "auto")) {
> pr_err("protocol %s not found\n", protocol);
> + mutex_unlock(&pi_mutex);
> return -EINVAL;
> }
> pr_wanted = NULL;
> } else {
> pr_wanted = container_of(drv, struct pi_protocol, driver);
> }
> -
> - mutex_lock(&pi_mutex);
> /* walk all parports */
> idr_for_each_entry(&parport_list, parport, port_num) {
> if (port_num == port_wanted || port_wanted == -1) {
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-07-30 3:16 [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store Pei Xiao
2026-07-30 3:29 ` sashiko-bot
2026-08-10 2:26 ` Pei Xiao
@ 2026-08-31 9:26 ` Niklas Cassel
2026-08-31 9:32 ` Niklas Cassel
2 siblings, 1 reply; 8+ messages in thread
From: Niklas Cassel @ 2026-08-31 9:26 UTC (permalink / raw)
To: dlemoal, linux, linux-ide, linux-kernel, Pei Xiao; +Cc: Shuangpeng Bai
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
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-08-31 9:26 ` Niklas Cassel
@ 2026-08-31 9:32 ` Niklas Cassel
2026-08-31 9:49 ` Pei Xiao
0 siblings, 1 reply; 8+ messages in thread
From: Niklas Cassel @ 2026-08-31 9:32 UTC (permalink / raw)
To: dlemoal, linux, linux-ide, linux-kernel, Pei Xiao; +Cc: Shuangpeng Bai
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."
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-08-31 9:32 ` Niklas Cassel
@ 2026-08-31 9:49 ` Pei Xiao
2026-08-31 10:36 ` Niklas Cassel
0 siblings, 1 reply; 8+ messages in thread
From: Pei Xiao @ 2026-08-31 9:49 UTC (permalink / raw)
To: Niklas Cassel, dlemoal, linux, linux-ide, linux-kernel; +Cc: Shuangpeng Bai
在 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-08-31 9:49 ` Pei Xiao
@ 2026-08-31 10:36 ` Niklas Cassel
2026-09-01 9:35 ` Pei Xiao
0 siblings, 1 reply; 8+ messages in thread
From: Niklas Cassel @ 2026-08-31 10:36 UTC (permalink / raw)
To: Pei Xiao; +Cc: dlemoal, linux, linux-ide, linux-kernel, Shuangpeng Bai
On Mon, Aug 31, 2026 at 05:49:51PM +0800, Pei Xiao wrote:
> 在 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/
That patch proposal seems to prevent the module from ever being unloaded:
https://lore.kernel.org/linux-ide/20260730082554.B2C161F00A3A@smtp.kernel.org/
So I would assume that it is not the right solution.
I did not try to unload the module myself while having devices attached.
However, as far as I can tell, this is a pre-existing problem, and not
related to your already accepted patch, so don't feel obligated to fix
this problem. It was mainly if you had some extra time.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store
2026-08-31 10:36 ` Niklas Cassel
@ 2026-09-01 9:35 ` Pei Xiao
0 siblings, 0 replies; 8+ messages in thread
From: Pei Xiao @ 2026-09-01 9:35 UTC (permalink / raw)
To: Niklas Cassel; +Cc: dlemoal, linux, linux-ide, linux-kernel, Shuangpeng Bai
在 2026/8/31 18:36, Niklas Cassel 写道:
> On Mon, Aug 31, 2026 at 05:49:51PM +0800, Pei Xiao wrote:
>> 在 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/
>
> That patch proposal seems to prevent the module from ever being unloaded:
> https://lore.kernel.org/linux-ide/20260730082554.B2C161F00A3A@smtp.kernel.org/
>
> So I would assume that it is not the right solution.
>
> I did not try to unload the module myself while having devices attached.
>
> However, as far as I can tell, this is a pre-existing problem, and not
> related to your already accepted patch, so don't feel obligated to fix
> this problem. It was mainly if you had some extra time.
hi Niklas,
Thank you for your reply. I'll take a look when I have some time.
Thanks!
Pei.
>
>
> Kind regards,
> Niklas
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-09-01 9:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 3:16 [PATCH v2] ata: pata_parport: Fix use-after-free in new_device_store Pei Xiao
2026-07-30 3:29 ` sashiko-bot
2026-08-10 2:26 ` Pei Xiao
2026-08-31 9:26 ` Niklas Cassel
2026-08-31 9:32 ` Niklas Cassel
2026-08-31 9:49 ` Pei Xiao
2026-08-31 10:36 ` Niklas Cassel
2026-09-01 9:35 ` Pei Xiao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox