* [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns()
@ 2023-03-14 6:20 Damien Le Moal
2023-03-14 18:41 ` Keith Busch
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Damien Le Moal @ 2023-03-14 6:20 UTC (permalink / raw)
To: linux-nvme, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni
Nvme specifications state that:
If the I/O Command Set associated with the namespace identified by the
NSID field does not support the Identify Namespace data structure
specified by the CSI field, the controller shall abort the command with
a status code of Invalid Field in Command.
In other words, if nvmet_execute_identify_cns_cs_ns() is called for a
target with a block device that is not zoned, we should not return any
data and set the status to NVME_SC_INVALID_FIELD.
While at it, it is also better to revalidate the ns block devie *before*
checking if the block device is zoned, to ensure that
nvmet_execute_identify_cns_cs_ns() operates against updated device
characteristics.
Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
drivers/nvme/target/zns.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 7e4292d88016..77ef0a86ff61 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -97,7 +97,7 @@ void nvmet_execute_identify_cns_cs_ctrl(struct nvmet_req *req)
void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
{
- struct nvme_id_ns_zns *id_zns;
+ struct nvme_id_ns_zns *id_zns = NULL;
u64 zsze;
u16 status;
u32 mar, mor;
@@ -118,16 +118,18 @@ void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
if (status)
goto done;
- if (!bdev_is_zoned(req->ns->bdev)) {
- req->error_loc = offsetof(struct nvme_identify, nsid);
- goto done;
- }
-
if (nvmet_ns_revalidate(req->ns)) {
mutex_lock(&req->ns->subsys->lock);
nvmet_ns_changed(req->ns->subsys, req->ns->nsid);
mutex_unlock(&req->ns->subsys->lock);
}
+
+ if (!bdev_is_zoned(req->ns->bdev)) {
+ status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+ req->error_loc = offsetof(struct nvme_identify, nsid);
+ goto out;
+ }
+
zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
req->ns->blksize_shift;
id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
@@ -148,8 +150,8 @@ void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
done:
status = nvmet_copy_to_sgl(req, 0, id_zns, sizeof(*id_zns));
- kfree(id_zns);
out:
+ kfree(id_zns);
nvmet_req_complete(req, status);
}
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns()
2023-03-14 6:20 [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns() Damien Le Moal
@ 2023-03-14 18:41 ` Keith Busch
2023-03-15 5:14 ` Chaitanya Kulkarni
2023-03-15 14:00 ` Christoph Hellwig
2 siblings, 0 replies; 5+ messages in thread
From: Keith Busch @ 2023-03-14 18:41 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-nvme, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni
On Tue, Mar 14, 2023 at 03:20:36PM +0900, Damien Le Moal wrote:
> Nvme specifications state that:
>
> If the I/O Command Set associated with the namespace identified by the
> NSID field does not support the Identify Namespace data structure
> specified by the CSI field, the controller shall abort the command with
> a status code of Invalid Field in Command.
>
> In other words, if nvmet_execute_identify_cns_cs_ns() is called for a
> target with a block device that is not zoned, we should not return any
> data and set the status to NVME_SC_INVALID_FIELD.
>
> While at it, it is also better to revalidate the ns block devie *before*
> checking if the block device is zoned, to ensure that
> nvmet_execute_identify_cns_cs_ns() operates against updated device
> characteristics.
>
> Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Looks good.
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns()
2023-03-14 6:20 [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns() Damien Le Moal
2023-03-14 18:41 ` Keith Busch
@ 2023-03-15 5:14 ` Chaitanya Kulkarni
2023-03-15 6:35 ` Damien Le Moal
2023-03-15 14:00 ` Christoph Hellwig
2 siblings, 1 reply; 5+ messages in thread
From: Chaitanya Kulkarni @ 2023-03-15 5:14 UTC (permalink / raw)
To: Damien Le Moal, linux-nvme, Christoph Hellwig, Sagi Grimberg,
Chaitanya Kulkarni
On 3/13/2023 11:20 PM, Damien Le Moal wrote:
> Nvme specifications state that:
>
> If the I/O Command Set associated with the namespace identified by the
> NSID field does not support the Identify Namespace data structure
> specified by the CSI field, the controller shall abort the command with
> a status code of Invalid Field in Command.
>
> In other words, if nvmet_execute_identify_cns_cs_ns() is called for a
> target with a block device that is not zoned, we should not return any
> data and set the status to NVME_SC_INVALID_FIELD.
>
> While at it, it is also better to revalidate the ns block devie *before*
> checking if the block device is zoned, to ensure that
> nvmet_execute_identify_cns_cs_ns() operates against updated device
> characteristics.
>
> Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
Thanks for fixing that.
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
-ck
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns()
2023-03-15 5:14 ` Chaitanya Kulkarni
@ 2023-03-15 6:35 ` Damien Le Moal
0 siblings, 0 replies; 5+ messages in thread
From: Damien Le Moal @ 2023-03-15 6:35 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-nvme, Christoph Hellwig, Sagi Grimberg
On 3/15/23 14:14, Chaitanya Kulkarni wrote:
> On 3/13/2023 11:20 PM, Damien Le Moal wrote:
>> Nvme specifications state that:
>>
>> If the I/O Command Set associated with the namespace identified by the
>> NSID field does not support the Identify Namespace data structure
>> specified by the CSI field, the controller shall abort the command with
>> a status code of Invalid Field in Command.
>>
>> In other words, if nvmet_execute_identify_cns_cs_ns() is called for a
>> target with a block device that is not zoned, we should not return any
>> data and set the status to NVME_SC_INVALID_FIELD.
>>
>> While at it, it is also better to revalidate the ns block devie *before*
>> checking if the block device is zoned, to ensure that
>> nvmet_execute_identify_cns_cs_ns() operates against updated device
>> characteristics.
>>
>> Fixes: aaf2e048af27 ("nvmet: add ZBD over ZNS backend support")
>> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
>> ---
>
> Thanks for fixing that.
More identify fixes coming.
>
> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Thanks.
>
> -ck
>
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns()
2023-03-14 6:20 [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns() Damien Le Moal
2023-03-14 18:41 ` Keith Busch
2023-03-15 5:14 ` Chaitanya Kulkarni
@ 2023-03-15 14:00 ` Christoph Hellwig
2 siblings, 0 replies; 5+ messages in thread
From: Christoph Hellwig @ 2023-03-15 14:00 UTC (permalink / raw)
To: Damien Le Moal
Cc: linux-nvme, Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni
Thanks,
applied to nvme-6.3.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-03-15 14:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-14 6:20 [PATCH] nvmet: Fix error handling in nvmet_execute_identify_cns_cs_ns() Damien Le Moal
2023-03-14 18:41 ` Keith Busch
2023-03-15 5:14 ` Chaitanya Kulkarni
2023-03-15 6:35 ` Damien Le Moal
2023-03-15 14:00 ` 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.