Linux-NVME Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] nvme: Fix parsing of ANA log page
@ 2019-10-28 22:56 Prabhath Sajeepa
  2019-10-29 17:15 ` Sagi Grimberg
  2019-10-30 14:29 ` Christoph Hellwig
  0 siblings, 2 replies; 6+ messages in thread
From: Prabhath Sajeepa @ 2019-10-28 22:56 UTC (permalink / raw)
  To: kbusch, axboe, hch, sagi, linux-nvme

Check validity of offset into ANA log buffer before accessing
nvme_ana_group_desc. This check ensures the size of ANA log buffer >=
offset + sizeof(nvme_ana_group_desc)

Signed-off-by: Prabhath Sajeepa <psajeepa@purestorage.com>
---
 drivers/nvme/host/multipath.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 30de7ef..cbd2b3d 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -444,8 +444,14 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
 
 	for (i = 0; i < le16_to_cpu(ctrl->ana_log_buf->ngrps); i++) {
 		struct nvme_ana_group_desc *desc = base + offset;
-		u32 nr_nsids = le32_to_cpu(desc->nnsids);
-		size_t nsid_buf_size = nr_nsids * sizeof(__le32);
+		u32 nr_nsids;
+		size_t nsid_buf_size;
+
+		if (WARN_ON_ONCE(offset > ctrl->ana_log_size - sizeof(*desc)))
+			return -EINVAL;
+
+		nr_nsids = le32_to_cpu(desc->nnsids);
+		nsid_buf_size = nr_nsids * sizeof(__le32);
 
 		if (WARN_ON_ONCE(desc->grpid == 0))
 			return -EINVAL;
@@ -465,8 +471,6 @@ static int nvme_parse_ana_log(struct nvme_ctrl *ctrl, void *data,
 			return error;
 
 		offset += nsid_buf_size;
-		if (WARN_ON_ONCE(offset > ctrl->ana_log_size - sizeof(*desc)))
-			return -EINVAL;
 	}
 
 	return 0;
-- 
2.7.4


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: Fix parsing of ANA log page
  2019-10-28 22:56 [PATCH] nvme: Fix parsing of ANA log page Prabhath Sajeepa
@ 2019-10-29 17:15 ` Sagi Grimberg
  2019-10-30  0:46   ` Keith Busch
  2019-10-30 14:29 ` Christoph Hellwig
  1 sibling, 1 reply; 6+ messages in thread
From: Sagi Grimberg @ 2019-10-29 17:15 UTC (permalink / raw)
  To: Prabhath Sajeepa, kbusch, axboe, hch, linux-nvme

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: Fix parsing of ANA log page
  2019-10-29 17:15 ` Sagi Grimberg
@ 2019-10-30  0:46   ` Keith Busch
  0 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2019-10-30  0:46 UTC (permalink / raw)
  To: Sagi Grimberg; +Cc: axboe, Prabhath Sajeepa, hch, linux-nvme

Thanks, applied to nvme-5.4. This is a new branch that was force pushed
to replace the previously defunct one with the same name.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: Fix parsing of ANA log page
  2019-10-28 22:56 [PATCH] nvme: Fix parsing of ANA log page Prabhath Sajeepa
  2019-10-29 17:15 ` Sagi Grimberg
@ 2019-10-30 14:29 ` Christoph Hellwig
  2019-10-30 19:55   ` Chaitanya Kulkarni
  1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2019-10-30 14:29 UTC (permalink / raw)
  To: Prabhath Sajeepa; +Cc: kbusch, axboe, hch, linux-nvme, sagi

The only thing this changes is that we now also execute the check
for the first entry parsed.  But the log size should never be so small
for this to matter.  Can you explain what problem your are trying to
solve?

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: Fix parsing of ANA log page
  2019-10-30 14:29 ` Christoph Hellwig
@ 2019-10-30 19:55   ` Chaitanya Kulkarni
  2019-10-30 23:39     ` Keith Busch
  0 siblings, 1 reply; 6+ messages in thread
From: Chaitanya Kulkarni @ 2019-10-30 19:55 UTC (permalink / raw)
  To: Christoph Hellwig, Prabhath Sajeepa; +Cc: kbusch, axboe, sagi, linux-nvme

On 10/30/2019 07:29 AM, Christoph Hellwig wrote:
> The only thing this changes is that we now also execute the check
> for the first entry parsed.  But the log size should never be so small
> for this to matter.  Can you explain what problem your are trying to
> solve?
>

I had hard time finding out that scenario.

Prabhat, can you please explain ?

> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
>


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] nvme: Fix parsing of ANA log page
  2019-10-30 19:55   ` Chaitanya Kulkarni
@ 2019-10-30 23:39     ` Keith Busch
  0 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2019-10-30 23:39 UTC (permalink / raw)
  To: Chaitanya Kulkarni
  Cc: axboe, Prabhath Sajeepa, Christoph Hellwig, linux-nvme, sagi

On Wed, Oct 30, 2019 at 07:55:44PM +0000, Chaitanya Kulkarni wrote:
> On 10/30/2019 07:29 AM, Christoph Hellwig wrote:
> > The only thing this changes is that we now also execute the check
> > for the first entry parsed.  But the log size should never be so small
> > for this to matter.  Can you explain what problem your are trying to
> > solve?
> >
> 
> I had hard time finding out that scenario.
> 
> Prabhat, can you please explain ?

The scenario should only apply if the controller reports an invalid
nanagrpid of 0, which spec disallows when ANA CMIC is set. We're not
validating nanagrpid compliance though, so this "fix" should only help
with broken controller implementations.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 22:56 [PATCH] nvme: Fix parsing of ANA log page Prabhath Sajeepa
2019-10-29 17:15 ` Sagi Grimberg
2019-10-30  0:46   ` Keith Busch
2019-10-30 14:29 ` Christoph Hellwig
2019-10-30 19:55   ` Chaitanya Kulkarni
2019-10-30 23:39     ` Keith Busch

Linux-NVME Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvme/0 linux-nvme/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvme linux-nvme/ https://lore.kernel.org/linux-nvme \
		linux-nvme@lists.infradead.org
	public-inbox-index linux-nvme

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-nvme


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git