linux-kernel - Re: [PATCH RESEND] scsi: ppa: Fix use-after-free caused by unfinished delayed work

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <389ac2dcc81b38367a26620cd193a45f2f06ce4f.camel@HansenPartnership.com>
Date: Sat, 03 Jan 2026 14:55:54 -0500
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: duoming@....edu.cn
Cc: linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	martin.petersen@...cle.com, stable@...nel.org
Subject: Re: [PATCH RESEND] scsi: ppa: Fix use-after-free caused by
 unfinished delayed work

On Sat, 2026-01-03 at 10:24 +0800, duoming@....edu.cn wrote:
> On Thu, 01 Jan 2026 10:21:28 -0500, James Bottomley wrote:
> > > diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
> > > index ea682f3044b..8da2a78ebac 100644
> > > --- a/drivers/scsi/ppa.c
> > > +++ b/drivers/scsi/ppa.c
> > > @@ -1136,6 +1136,7 @@ static void ppa_detach(struct parport *pb)
> > >  	ppa_struct *dev;
> > >  	list_for_each_entry(dev, &ppa_hosts, list) {
> > >  		if (dev->dev->port == pb) {
> > > +			disable_delayed_work_sync(&dev->ppa_tq);
> > >  			list_del_init(&dev->list);
> > >  			scsi_remove_host(dev->host);
> > >  			scsi_host_put(dev->host);
> > 
> > This fix looks bogus: if there's an active workqueue on ppa it's
> > because there's an outstanding command and it's emulating polling. 
> > If you stop the polling by disabling the workqueue, the command
> > will never return and the host will never get freed, so this will
> > leak resources, won't it?
> 
> I believe that disabling the ppa_tq delayed work will not affect the
> Scsi_Host release process. The lifetime of the Scsi_Host is managed
> by tagset_refcnt. The tagset_refcnt is initialized to 1 in
> scsi_add_host_with_dma() and decreased to 0 in scsi_remove_host().
> since the delayed work callback ppa_interrupt() does not modify
> tagset_refcnt in any way, the host could be freed as expected.

Not if something else holds a reference to the host, which an
outstanding command does.  That's the point I was making above: as long
as the command doesn't return, everything is pinned and never gets
freed (well, possibly until timeout).  You cause that because the work
queue is only active if a command is outstanding, so when you disable
the queue in that situation the command remains outstanding and can
never complete normally.

> > Also the race condition you identify is one of many tied to an
> > incorrect ppa_struct lifetime: it should never be free'd before the
> > host itself is gone because a live host may do a callback which
> > will get the ppa_struct from hostdata, so if the host is still
> > alive for any reason when ppa_detach() is called, we'll get the
> > same problem.
> 
> The ppa_struct is properly freed only after ensuring the complete
> removal of the associated Scsi_Host in ppa_detach(). The detailed
> sequence is as follows:
> 
> ...
>    scsi_remove_host(dev->host);
>    scsi_host_put(dev->host); //the host is gone

The host is not gone if that put is not the final one as it won't be if
there's an outstanding command pinning the device.

Regards,

James