* [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