Re: [PATCH v2 2/2] ata: pata_parport: unregister devices on protocol unregister

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] pata_parport_attach fails for all parports except port 0
--

commit b043c5b13721ce815e34642853318549a000ffd3
Author: Pei Xiao <xiaopei01@kylinos.cn>

ata: pata_parport: unregister devices on protocol unregister

When a protocol module registers multiple protocols and a later
registration fails, the rollback path unregisters the protocol but leaves
dynamically created devices untouched. Later removal of these dangling
devices dereferences freed module memory, crashing the kernel. This commit
tears down all pi_adapters using the protocol before unregistering the
driver.

> diff --git a/drivers/ata/pata_parport/pata_parport.c b/drivers/ata/pata_parport/pata_parport.c
> index 046ab7e3adbcf..712209882ab75 100644
> --- a/drivers/ata/pata_parport/pata_parport.c
> +++ b/drivers/ata/pata_parport/pata_parport.c
[ ... ]

While reviewing this, I noticed a pre-existing issue in pata_parport_attach():

drivers/ata/pata_parport/pata_parport.c:pata_parport_attach() {
    ...
	mutex_lock(&pi_mutex);
	id = idr_alloc(&parport_list, port, port->number, port->number,
		       GFP_KERNEL);
	if (id < 0) {
    ...
}

[Severity: High]
This is a pre-existing issue, but does this idr_alloc() call fail for all
parports except port 0?

Since the end parameter of idr_alloc() is exclusive, when port->number > 0,
it computes max = end - 1. Because start is equal to port->number, the
condition start > max evaluates to true, causing it to return -ENOSPC.

Would this cause attaching any parport with a number greater than 0 to
silently fail and completely break functionality for those ports?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788867690.git.xiaopei01@kylinos.cn?part=2