* [PATCH] nvme: fix potential invalid pointer dereference
@ 2023-01-27 15:42 Maurizio Lombardi
2023-01-27 15:58 ` Keith Busch
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Maurizio Lombardi @ 2023-01-27 15:42 UTC (permalink / raw)
To: kbusch; +Cc: axboe, hch, sagi, linux-nvme
If nvme_alloc_admin_tag_set() fails, the admin_q and fabrics_q pointers
are left with an invalid, non-NULL value;
Other functions may then check the pointers' value and dereference them,
like it happens in
nvme_probe() -> out_disable: -> nvme_dev_remove_admin().
Fix the bug by setting admin_q and fabrics_q to NULL in case of error.
Also fix a NULL pointer dereference (the ctrl->admin_tagset pointer
is only initialized just before returning success;
therefore, in the error code path, blk_mq_free_tag_set() must be
called against the "set" variable).
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
---
drivers/nvme/host/core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 7be562a4e1aa..6b277b80a09c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -4921,7 +4921,9 @@ int nvme_alloc_admin_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set,
blk_mq_destroy_queue(ctrl->admin_q);
blk_put_queue(ctrl->admin_q);
out_free_tagset:
- blk_mq_free_tag_set(ctrl->admin_tagset);
+ blk_mq_free_tag_set(set);
+ ctrl->admin_q = NULL;
+ ctrl->fabrics_q = NULL;
return ret;
}
EXPORT_SYMBOL_GPL(nvme_alloc_admin_tag_set);
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: fix potential invalid pointer dereference
2023-01-27 15:42 [PATCH] nvme: fix potential invalid pointer dereference Maurizio Lombardi
@ 2023-01-27 15:58 ` Keith Busch
2023-01-30 10:05 ` Christoph Hellwig
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2023-01-27 15:58 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: axboe, hch, sagi, linux-nvme
On Fri, Jan 27, 2023 at 04:42:37PM +0100, Maurizio Lombardi wrote:
> If nvme_alloc_admin_tag_set() fails, the admin_q and fabrics_q pointers
> are left with an invalid, non-NULL value;
> Other functions may then check the pointers' value and dereference them,
> like it happens in
> nvme_probe() -> out_disable: -> nvme_dev_remove_admin().
>
> Fix the bug by setting admin_q and fabrics_q to NULL in case of error.
> Also fix a NULL pointer dereference (the ctrl->admin_tagset pointer
> is only initialized just before returning success;
> therefore, in the error code path, blk_mq_free_tag_set() must be
> called against the "set" variable).
Looks good.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: fix potential invalid pointer dereference
2023-01-27 15:42 [PATCH] nvme: fix potential invalid pointer dereference Maurizio Lombardi
2023-01-27 15:58 ` Keith Busch
@ 2023-01-30 10:05 ` Christoph Hellwig
2023-01-30 10:13 ` Maurizio Lombardi
2023-01-31 21:11 ` Chaitanya Kulkarni
2023-02-01 13:19 ` Christoph Hellwig
3 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2023-01-30 10:05 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: kbusch, axboe, hch, sagi, linux-nvme
nvme_alloc_io_tag_set needs the same fix.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: fix potential invalid pointer dereference
2023-01-30 10:05 ` Christoph Hellwig
@ 2023-01-30 10:13 ` Maurizio Lombardi
0 siblings, 0 replies; 6+ messages in thread
From: Maurizio Lombardi @ 2023-01-30 10:13 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kbusch, axboe, hch, sagi, linux-nvme
po 30. 1. 2023 v 11:06 odesílatel Christoph Hellwig <hch@infradead.org> napsal:
>
> nvme_alloc_io_tag_set needs the same fix.
>
Yep, I'm going to send a patch.
Maurizio
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: fix potential invalid pointer dereference
2023-01-27 15:42 [PATCH] nvme: fix potential invalid pointer dereference Maurizio Lombardi
2023-01-27 15:58 ` Keith Busch
2023-01-30 10:05 ` Christoph Hellwig
@ 2023-01-31 21:11 ` Chaitanya Kulkarni
2023-02-01 13:19 ` Christoph Hellwig
3 siblings, 0 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2023-01-31 21:11 UTC (permalink / raw)
To: Maurizio Lombardi, kbusch@kernel.org
Cc: axboe@fb.com, hch@lst.de, sagi@grimberg.me,
linux-nvme@lists.infradead.org
On 1/27/23 07:42, Maurizio Lombardi wrote:
> If nvme_alloc_admin_tag_set() fails, the admin_q and fabrics_q pointers
> are left with an invalid, non-NULL value;
> Other functions may then check the pointers' value and dereference them,
> like it happens in
> nvme_probe() -> out_disable: -> nvme_dev_remove_admin().
>
> Fix the bug by setting admin_q and fabrics_q to NULL in case of error.
> Also fix a NULL pointer dereference (the ctrl->admin_tagset pointer
> is only initialized just before returning success;
> therefore, in the error code path, blk_mq_free_tag_set() must be
> called against the "set" variable).
>
> Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
> ---
Looks good.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] nvme: fix potential invalid pointer dereference
2023-01-27 15:42 [PATCH] nvme: fix potential invalid pointer dereference Maurizio Lombardi
` (2 preceding siblings ...)
2023-01-31 21:11 ` Chaitanya Kulkarni
@ 2023-02-01 13:19 ` Christoph Hellwig
3 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2023-02-01 13:19 UTC (permalink / raw)
To: Maurizio Lombardi; +Cc: kbusch, axboe, hch, sagi, linux-nvme
Thanks,
applied to nvme-6.2.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-01 13:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-27 15:42 [PATCH] nvme: fix potential invalid pointer dereference Maurizio Lombardi
2023-01-27 15:58 ` Keith Busch
2023-01-30 10:05 ` Christoph Hellwig
2023-01-30 10:13 ` Maurizio Lombardi
2023-01-31 21:11 ` Chaitanya Kulkarni
2023-02-01 13:19 ` Christoph Hellwig
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.