[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <46574732.55570.19b91cdc1e4.Coremail.duoming@zju.edu.cn>
Date: Tue, 6 Jan 2026 13:35:39 +0800 (GMT+08:00)
From: duoming@....edu.cn
To: "James Bottomley" <James.Bottomley@...senPartnership.com>
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 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
Powered by blists - more mailing lists