From: Ming Lei <ming.lei@redhat.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: 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>,
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>
Subject: Re: [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests
Date: Mon, 26 Apr 2021 09:19:45 +0800 [thread overview]
Message-ID: <YIYVMQmAZgKL2qdP@T590> (raw)
In-Reply-To: <5c1ef3ec-dd6a-4992-586b-6e67bcd1a678@acm.org>
On Sun, Apr 25, 2021 at 01:53:16PM -0700, Bart Van Assche wrote:
> On 4/25/21 2:27 AM, Ming Lei wrote:
> > On Sun, Apr 25, 2021 at 04:57:45PM +0800, Ming Lei wrote:
> >> Revert 4 patches from Bart which try to fix request UAF issue related
> >> with iterating over tagset wide requests, because:
>
> Where were you during the four weeks that my patch series was out for
> review? I haven't seen any feedback from you on my patch series.
To be honest, it is just two days ago I have to take a close look
at your patchset because we may have to backport your patches for
addressing one RH report with high priority.
David is in CC list, and Laurence/David is looking the report too.
>
> >> 1) request UAF caused by normal completion vs. async completion during
> >> iterating can't be covered[1]
>
> I do not agree with the above. Patches 5/8 and 6/8 from this series can
> be applied without reverting any of my patches.
The thing is that 5 ~ 8 can fix the issue in a simpler way without
adding extra cost in fast path, and the idea is easier to be proved.
BTW, as a downstream kernel developer, I really hope all fix are simple and
easy to backport. More importantly, I do prefer to approaches in patch which
can be proved/verified easily, so further regression can be avoided.
>
> > 4) synchronize_rcu() is added before shutting down one request queue,
> > which may slow down reboot/poweroff very much on big systems with lots of
> > HBAs in which lots of LUNs are attached.
>
> The synchronize_rcu() can be removed by using a semaphore
> (<linux/semaphore.h>) instead of an RCU reader lock inside bt_tags_iter().
I am not sure you can, because some iteration is done in atomic context.
Thanks,
Ming
WARNING: multiple messages have this Message-ID (diff)
From: Ming Lei <ming.lei@redhat.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: 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>,
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>
Subject: Re: [PATCH 0/8] blk-mq: fix request UAF related with iterating over tagset requests
Date: Mon, 26 Apr 2021 09:19:45 +0800 [thread overview]
Message-ID: <YIYVMQmAZgKL2qdP@T590> (raw)
In-Reply-To: <5c1ef3ec-dd6a-4992-586b-6e67bcd1a678@acm.org>
On Sun, Apr 25, 2021 at 01:53:16PM -0700, Bart Van Assche wrote:
> On 4/25/21 2:27 AM, Ming Lei wrote:
> > On Sun, Apr 25, 2021 at 04:57:45PM +0800, Ming Lei wrote:
> >> Revert 4 patches from Bart which try to fix request UAF issue related
> >> with iterating over tagset wide requests, because:
>
> Where were you during the four weeks that my patch series was out for
> review? I haven't seen any feedback from you on my patch series.
To be honest, it is just two days ago I have to take a close look
at your patchset because we may have to backport your patches for
addressing one RH report with high priority.
David is in CC list, and Laurence/David is looking the report too.
>
> >> 1) request UAF caused by normal completion vs. async completion during
> >> iterating can't be covered[1]
>
> I do not agree with the above. Patches 5/8 and 6/8 from this series can
> be applied without reverting any of my patches.
The thing is that 5 ~ 8 can fix the issue in a simpler way without
adding extra cost in fast path, and the idea is easier to be proved.
BTW, as a downstream kernel developer, I really hope all fix are simple and
easy to backport. More importantly, I do prefer to approaches in patch which
can be proved/verified easily, so further regression can be avoided.
>
> > 4) synchronize_rcu() is added before shutting down one request queue,
> > which may slow down reboot/poweroff very much on big systems with lots of
> > HBAs in which lots of LUNs are attached.
>
> The synchronize_rcu() can be removed by using a semaphore
> (<linux/semaphore.h>) instead of an RCU reader lock inside bt_tags_iter().
I am not sure you can, because some iteration is done in atomic context.
Thanks,
Ming
_______________________________________________
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-26 1:19 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 ` [PATCH 6/8] block: drivers: complete request locally from blk_mq_tagset_busy_iter Ming Lei
2021-04-25 8:57 ` 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 [this message]
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=YIYVMQmAZgKL2qdP@T590 \
--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: link
Be 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.