On Mon, Apr 26, 2021 at 02:24:56PM +0800, Ming Lei wrote: > On Sun, Apr 25, 2021 at 08:02:10PM -0700, Bart Van Assche wrote: > > On 4/25/21 1:57 AM, Ming Lei wrote: > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > > index c289991ffaed..7cbaee282b6d 100644 > > > --- a/drivers/scsi/scsi_lib.c > > > +++ b/drivers/scsi/scsi_lib.c > > > @@ -1568,7 +1568,11 @@ static void scsi_mq_done(struct scsi_cmnd *cmd) > > > if (unlikely(test_and_set_bit(SCMD_STATE_COMPLETE, &cmd->state))) > > > return; > > > trace_scsi_dispatch_cmd_done(cmd); > > > - blk_mq_complete_request(cmd->request); > > > + > > > + if (unlikely(host_byte(cmd->result) != DID_OK)) > > > + blk_mq_complete_request_locally(cmd->request); > > > + else > > > + blk_mq_complete_request(cmd->request); > > > } > > > > This change is so tricky that it deserves a comment. > > > > An even better approach would be *not* to export > > blk_mq_complete_request_locally() from the block layer to block drivers > > and instead modify the block layer such that it completes a request on > > the same CPU if request completion happens from inside the context of a > > tag iteration function. That would save driver writers the trouble of > > learning yet another block layer API. > > Yeah, that is possible, and one request flag(eg. RQF_ITERATED) can be added. > The flag is set before calling ->fn(), and evaluated in > blk_mq_complete_request_remote(). Thinking of the issue further, we have grabbed rq->refcnt before calling ->fn(), not only request UAF is fixed, but also driver gets valid request instance to check if the request has been completed, so we needn't to consider the double completion issue[1] any more, which is supposed to be covered by driver, such as, scsi uses SCMD_STATE_COMPLETE in scsi_mq_done() for avoiding double completion. [1] https://lore.kernel.org/linux-block/YIdWz8C5eklPvEiV@T590/T/#u Thanks, Ming
