From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: duoming@zju.edu.cn
Cc: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
martin.petersen@oracle.com, stable@kernel.org
Subject: Re: [PATCH RESEND] scsi: ppa: Fix use-after-free caused by unfinished delayed work
Date: Tue, 06 Jan 2026 22:35:20 -0500 [thread overview]
Message-ID: <159a881fd0393cc390a4597a1d0c39b1903fe906.camel@HansenPartnership.com> (raw)
In-Reply-To: <46574732.55570.19b91cdc1e4.Coremail.duoming@zju.edu.cn>
On Tue, 2026-01-06 at 13:35 +0800, duoming@zju.edu.cn wrote:
> On Sun, 04 Jan 2026 18:30:48 -0500 James Bottomley wrote:
[...]
> > 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.
Well, no, because that would lose us runtime error handling which is
pretty essential for a flakey device like parport. Also, if you remove
that callback, it will escalate to the reset handler, which does
exactly the same thing with ppa_struct.
I was originally looking at this as a reference counting problem, but
there is another way of solving it and that's to stop requests and
drain the queue before freeing the resources. It turns out that's
exactly what scsi_remove_host() does (via __scsi_remove_device which
calls blk_mq_destroy_queue) so, since there can't be any outstanding
commands, it's actually impossible the delayed work queue is active
after scsi_remove_host() is called and thus the original race you
identified can't actually happen.
Regards,
James
prev parent reply other threads:[~2026-01-07 3:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-01-01 13:55 Duoming Zhou
2026-01-01 15:21 ` James Bottomley
2026-01-03 2:24 ` duoming
2026-01-03 19:55 ` James Bottomley
2026-01-04 14:35 ` duoming
2026-01-04 23:30 ` James Bottomley
2026-01-06 5:35 ` duoming
2026-01-07 3:35 ` James Bottomley [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=159a881fd0393cc390a4597a1d0c39b1903fe906.camel@HansenPartnership.com \
--to=james.bottomley@hansenpartnership.com \
--cc=duoming@zju.edu.cn \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=stable@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®