From: Ming Lei <ming.lei@redhat.com> To: linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org, Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org, "Martin K . Petersen" <martin.petersen@oracle.com>, Christoph Hellwig <hch@lst.de> Cc: Bart Van Assche <bvanassche@acm.org>, Khazhy Kumykov <khazhy@google.com>, Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>, Hannes Reinecke <hare@suse.de>, John Garry <john.garry@huawei.com>, David Jeffery <djeffery@redhat.com>, Ming Lei <ming.lei@redhat.com> Subject: [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter Date: Sun, 25 Apr 2021 16:57:51 +0800 [thread overview] Message-ID: <20210425085753.2617424-7-ming.lei@redhat.com> (raw) In-Reply-To: <20210425085753.2617424-1-ming.lei@redhat.com> It can be a bit hard for driver to avoid request UAF between normal completion and completion via blk_mq_tagset_busy_iter() if async completion is done in blk_mq_tagset_busy_iter(). Cause request->tag is only freed after .mq_ops->complete() is called, and rquest->tag may still be valid after blk_mq_complete_request() is returned from normal completion path, so this request is still visible in blk_mq_tagset_busy_iter(). This patch itself can't avoid such request UAF completely. We will grab a request reference in next patch when walking request via blk_mq_tagset_busy_iter() for fixing such race, that is why we have to convert to blk_mq_complete_request_locally() first. Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/mtip32xx/mtip32xx.c | 2 +- drivers/block/nbd.c | 2 +- drivers/nvme/host/core.c | 2 +- drivers/scsi/scsi_lib.c | 6 +++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 3be0dbc674bd..05f5e36ee608 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -3748,7 +3748,7 @@ static bool mtip_no_dev_cleanup(struct request *rq, void *data, bool reserv) struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); cmd->status = BLK_STS_IOERR; - blk_mq_complete_request(rq); + blk_mq_complete_request_locally(rq); return true; } diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4ff71b579cfc..3dcf3288efa8 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -809,7 +809,7 @@ static bool nbd_clear_req(struct request *req, void *data, bool reserved) cmd->status = BLK_STS_IOERR; mutex_unlock(&cmd->lock); - blk_mq_complete_request(req); + blk_mq_complete_request_locally(req); return true; } diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 0896e21642be..a605954477da 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -381,7 +381,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved) nvme_req(req)->status = NVME_SC_HOST_ABORTED_CMD; nvme_req(req)->flags |= NVME_REQ_CANCELLED; - blk_mq_complete_request(req); + blk_mq_complete_request_locally(req); return true; } EXPORT_SYMBOL_GPL(nvme_cancel_request); 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); } static void scsi_mq_put_budget(struct request_queue *q) -- 2.29.2
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <ming.lei@redhat.com> To: linux-nvme@lists.infradead.org, linux-scsi@vger.kernel.org, Jens Axboe <axboe@kernel.dk>, linux-block@vger.kernel.org, "Martin K . Petersen" <martin.petersen@oracle.com>, Christoph Hellwig <hch@lst.de> Cc: Bart Van Assche <bvanassche@acm.org>, Khazhy Kumykov <khazhy@google.com>, Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>, Hannes Reinecke <hare@suse.de>, John Garry <john.garry@huawei.com>, David Jeffery <djeffery@redhat.com>, Ming Lei <ming.lei@redhat.com> Subject: [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter Date: Sun, 25 Apr 2021 16:57:51 +0800 [thread overview] Message-ID: <20210425085753.2617424-7-ming.lei@redhat.com> (raw) In-Reply-To: <20210425085753.2617424-1-ming.lei@redhat.com> It can be a bit hard for driver to avoid request UAF between normal completion and completion via blk_mq_tagset_busy_iter() if async completion is done in blk_mq_tagset_busy_iter(). Cause request->tag is only freed after .mq_ops->complete() is called, and rquest->tag may still be valid after blk_mq_complete_request() is returned from normal completion path, so this request is still visible in blk_mq_tagset_busy_iter(). This patch itself can't avoid such request UAF completely. We will grab a request reference in next patch when walking request via blk_mq_tagset_busy_iter() for fixing such race, that is why we have to convert to blk_mq_complete_request_locally() first. Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/mtip32xx/mtip32xx.c | 2 +- drivers/block/nbd.c | 2 +- drivers/nvme/host/core.c | 2 +- drivers/scsi/scsi_lib.c | 6 +++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/block/mtip32xx/mtip32xx.c b/drivers/block/mtip32xx/mtip32xx.c index 3be0dbc674bd..05f5e36ee608 100644 --- a/drivers/block/mtip32xx/mtip32xx.c +++ b/drivers/block/mtip32xx/mtip32xx.c @@ -3748,7 +3748,7 @@ static bool mtip_no_dev_cleanup(struct request *rq, void *data, bool reserv) struct mtip_cmd *cmd = blk_mq_rq_to_pdu(rq); cmd->status = BLK_STS_IOERR; - blk_mq_complete_request(rq); + blk_mq_complete_request_locally(rq); return true; } diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 4ff71b579cfc..3dcf3288efa8 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -809,7 +809,7 @@ static bool nbd_clear_req(struct request *req, void *data, bool reserved) cmd->status = BLK_STS_IOERR; mutex_unlock(&cmd->lock); - blk_mq_complete_request(req); + blk_mq_complete_request_locally(req); return true; } diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 0896e21642be..a605954477da 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -381,7 +381,7 @@ bool nvme_cancel_request(struct request *req, void *data, bool reserved) nvme_req(req)->status = NVME_SC_HOST_ABORTED_CMD; nvme_req(req)->flags |= NVME_REQ_CANCELLED; - blk_mq_complete_request(req); + blk_mq_complete_request_locally(req); return true; } EXPORT_SYMBOL_GPL(nvme_cancel_request); 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); } static void scsi_mq_put_budget(struct request_queue *q) -- 2.29.2 _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2021-04-25 8:58 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-25 8:57 [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` [PATCH 1/8] Revert "blk-mq: Fix races between blk_mq_update_nr_hw_queues() and iterating over tags" Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` [PATCH 2/8] Revert "blk-mq: Make it safe to use RCU to iterate over blk_mq_tag_set.tag_list" Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` [PATCH 3/8] Revert "blk-mq: Fix races between iterating over requests and freeing requests" Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` [PATCH 4/8] Revert "blk-mq: Introduce atomic variants of blk_mq_(all_tag|tagset_busy)_iter" Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` [PATCH 5/8] blk-mq: blk_mq_complete_request_locally Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 8:57 ` Ming Lei [this message] 2021-04-25 8:57 ` [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter Ming Lei 2021-04-26 3:02 ` Bart Van Assche 2021-04-26 3:02 ` Bart Van Assche 2021-04-26 6:24 ` Ming Lei 2021-04-26 6:24 ` Ming Lei 2021-04-27 8:54 ` Ming Lei 2021-04-27 8:54 ` Ming Lei 2021-04-25 8:57 ` [PATCH 7/8] blk-mq: grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 18:55 ` Bart Van Assche 2021-04-25 18:55 ` Bart Van Assche 2021-04-26 0:41 ` Ming Lei 2021-04-26 0:41 ` Ming Lei 2021-04-25 8:57 ` [PATCH 8/8] blk-mq: clear stale request in tags->rq[] before freeing one request pool Ming Lei 2021-04-25 8:57 ` Ming Lei 2021-04-25 20:42 ` Bart Van Assche 2021-04-25 20:42 ` Bart Van Assche 2021-04-26 0:49 ` Ming Lei 2021-04-26 0:49 ` Ming Lei 2021-04-26 1:50 ` Bart Van Assche 2021-04-26 1:50 ` Bart Van Assche 2021-04-26 2:07 ` Ming Lei 2021-04-26 2:07 ` Ming Lei 2021-04-25 9:27 ` [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests Ming Lei 2021-04-25 9:27 ` Ming Lei 2021-04-25 20:53 ` Bart Van Assche 2021-04-25 20:53 ` Bart Van Assche 2021-04-26 1:19 ` Ming Lei 2021-04-26 1:19 ` Ming Lei 2021-04-26 1:57 ` Bart Van Assche 2021-04-26 1:57 ` Bart Van Assche 2021-04-25 16:17 ` Jens Axboe 2021-04-25 16:17 ` Jens Axboe 2021-04-25 18:39 ` Bart Van Assche 2021-04-25 18:39 ` Bart Van Assche 2021-04-25 20:18 ` Jens Axboe 2021-04-25 20:18 ` Jens Axboe
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=20210425085753.2617424-7-ming.lei@redhat.com \ --to=ming.lei@redhat.com \ --cc=axboe@kernel.dk \ --cc=bvanassche@acm.org \ --cc=djeffery@redhat.com \ --cc=hare@suse.de \ --cc=hch@lst.de \ --cc=john.garry@huawei.com \ --cc=khazhy@google.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=linux-scsi@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=shinichiro.kawasaki@wdc.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.