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

[duoming@zju.edu.cn]

On Sun, 04 Jan 2026 18:30:48 -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.
> > 
> > When a request is sent to a SCSI device, a timer is set for the
> > request. If it times out, the request is directly terminated and
> > removed from the queue. The specific function call chain is as
> > follows: 
> 
> I know how timeouts work ... I don't need the AI summary.  if there is
> an outstanding command, the timeout will fire long after you've
> disabled the queue and run through ppa_detach and the next thing that
> will happen is the error handler will try to abort the command,
> eventually causing ppa_abort to be called, which is going to
> dereference the ppa_struct that ppa_detach freed.

What do you think of removing the ppa_abort() callback function? 
This callback function is not necessary. The eh_abort_handler 
function pointer is checked in scsi_abort_command() function, 
and if the function pointer does not exist, it directly returns 
FAILED. The subsequent process will be handled by SCSI error 
handler thread - scsi_error_handler(). Therefore the issue you
mentioned could be avoided.

Best regards,
Duoming Zhou