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

[Niklas Cassel <cassel@kernel.org>]

On Tue, Sep 08, 2026 at 12:08:55PM +0000, sashiko-bot@kernel.org wrote:
> 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?

Looking at idr_alloc(), this looks like a legit pre-existing issue.

There is another call to idr_alloc() i pata_parport_register_driver(),
but it does:
error = idr_alloc(&protocols, pr, 0, 0, GFP_KERNEL);

0,0 is start,end

if @end is <= 0, it is treated as one larger than %INT_MAX

So this is only a problem in pata_parport_attach().



Kind regards,
Niklas