[PATCH] ata: pata_parport: Fix missing module refcount for pata_parport itself

[PATCH] ata: pata_parport: Fix missing module refcount for pata_parport itself

[Pei Xiao]

The pata_parport core module can be unloaded while devices are still
attached, because there is no module reference taken for the core
module itself -- only for the protocol driver.

Add a __module_get(THIS_MODULE) in the success path of pi_init_one()
and the corresponding module_put(THIS_MODULE) in
pata_parport_dev_release(), so that rmmod pata_parport is prevented
when devices are present.

In the dev_release callback, the pi->proto guard ensures that
module_put() is skipped for the early error case where
device_register() fails before __module_get() was called.

Fixes: 246a1c4c6b7f ("ata: pata_parport: add driver (PARIDE replacement)")
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/ata/pata_parport/pata_parport.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
index cf81a6128f55..b73b023b0443 100644
--- a/drivers/ata/pata_parport/pata_parport.c
+++ b/drivers/ata/pata_parport/pata_parport.c
@@ -455,6 +455,12 @@ static void pata_parport_dev_release(struct device *dev)
 {
 	struct pi_adapter *pi = container_of(dev, struct pi_adapter, dev);
 
+	/*
+	 * pi->proto is NULL when device_register() fails early,
+	 * before __module_get() was called -- skip module_put().
+	 */
+	if (pi->proto)
+		module_put(THIS_MODULE);
 	ida_free(&pata_parport_bus_dev_ids, dev->id);
 	kfree(pi);
 }
@@ -524,6 +530,8 @@ static struct pi_adapter *pi_init_one(struct parport *parport,
 
 	pi->proto = pr;
 
+	__module_get(THIS_MODULE);
+
 	if (!try_module_get(pi->proto->owner))
 		goto out_unreg_dev;
 	if (pi->proto->init_proto && pi->proto->init_proto(pi) < 0)
-- 
2.25.1

Re: [PATCH] ata: pata_parport: Fix missing module refcount for pata_parport itself

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Circular dependency permanently preventing module unloading.
- [High] Race condition leading to Use-After-Free during device release.
--

--- Patch [1]: [PATCH] ata: pata_parport: Fix missing module refcount for pata_parport itself ---
commit 35d67ec0c667d20f10f4b6d490918bd71ccafa32
Author: Pei Xiao <xiaopei01@kylinos.cn>

ata: pata_parport: Fix missing module refcount for pata_parport itself

This commit adds a module reference count for the pata_parport core module
during device initialization and drops it during device release. This is
intended to prevent the module from being unloaded while devices are still
attached.

> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> index 40baeac594a9f..6de237602a13e 100644
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
> @@ -455,6 +455,12 @@ static void pata_parport_dev_release(struct device *dev)
>  {
>  	struct pi_adapter *pi = container_of(dev, struct pi_adapter, dev);
>  
> +	/*
> +	 * pi->proto is NULL when device_register() fails early,
> +	 * before __module_get() was called -- skip module_put().
> +	 */
> +	if (pi->proto)
> +		module_put(THIS_MODULE);

[Severity: High]
Does this create a race condition where the module could be unloaded while
pata_parport_dev_release() is still executing?

If module_put() drops the final reference to the module, a concurrent module
unload could unmap the module's memory. The remaining instructions in this
function, including the call to ida_free() which accesses the static
pata_parport_bus_dev_ids structure, would then be executing in freed memory.

>  	ida_free(&pata_parport_bus_dev_ids, dev->id);
>  	kfree(pi);
>  }

[ ... ]

> @@ -524,6 +530,8 @@ static struct pi_adapter *pi_init_one(struct parport *parport,
>  
>  	pi->proto = pr;
>  
> +	__module_get(THIS_MODULE);
> +

[Severity: High]
Will this prevent the module from ever being unloaded?

Normally, the driver's exit function (pata_parport_exit) would unregister the
driver, which in turn detaches devices and drops their references. By taking a
module reference for every attached device, the module reference count will
never reach zero while devices exist, making pata_parport_exit() unreachable
via standard unload operations.

>  	if (!try_module_get(pi->proto->owner))
>  		goto out_unreg_dev;
>  	if (pi->proto->init_proto && pi->proto->init_proto(pi) < 0)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/a8d9807618ab786313654099c904d613075e1b63.1785398816.git.xiaopei01@kylinos.cn?part=1