[PATCH v2 for-4.18 1/2] nvmet: fix file discard return status

[PATCH v2 for-4.18 1/2] nvmet: fix file discard return status

[Sagi Grimberg]

If nvmet_copy_from_sgl failed, we falsly return successful
completion status.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
Changes from v1:
- make ret type u16

 drivers/nvme/target/io-cmd-file.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 57c660e3245d..dad8d44bf90e 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -227,22 +227,24 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
 {
 	int mode = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
 	struct nvme_dsm_range range;
-	loff_t offset;
-	loff_t len;
-	int i, ret;
+	loff_t offset, len;
+	u16 ret;
+	int i;
 
 	for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
-		if (nvmet_copy_from_sgl(req, i * sizeof(range), &range,
-					sizeof(range)))
+		ret = nvmet_copy_from_sgl(req, i * sizeof(range), &range,
+					sizeof(range));
+		if (ret)
 			break;
 		offset = le64_to_cpu(range.slba) << req->ns->blksize_shift;
 		len = le32_to_cpu(range.nlb) << req->ns->blksize_shift;
-		ret = vfs_fallocate(req->ns->file, mode, offset, len);
-		if (ret)
+		if (vfs_fallocate(req->ns->file, mode, offset, len)) {
+			ret = NVME_SC_INTERNAL | NVME_SC_DNR;
 			break;
+		}
 	}
 
-	nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
+	nvmet_req_complete(req, ret);
 }
 
 static void nvmet_file_dsm_work(struct work_struct *w)
-- 
2.14.1

[PATCH v2 for-4.18 2/2] nvmet: check fileio lba range access boundaries

[Sagi Grimberg]

Fail out-of-bounds with a proper status code.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
Changes from v1:
- move boundary check before resource allocation so we don't need
to worry about freeing them (nvmet_file_execute_rw).

 drivers/nvme/target/io-cmd-file.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index dad8d44bf90e..c2541ebdac69 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -145,6 +145,12 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
 		return;
 	}
 
+	pos = le64_to_cpu(req->cmd->rw.slba) << req->ns->blksize_shift;
+	if (unlikely(pos + req->data_len > req->ns->size)) {
+		nvmet_req_complete(req, NVME_SC_LBA_RANGE | NVME_SC_DNR);
+		return;
+	}
+
 	if (nr_bvec > NVMET_MAX_INLINE_BIOVEC)
 		req->f.bvec = kmalloc_array(nr_bvec, sizeof(struct bio_vec),
 				GFP_KERNEL);
@@ -160,8 +166,6 @@ static void nvmet_file_execute_rw(struct nvmet_req *req)
 			is_sync = true;
 	}
 
-	pos = le64_to_cpu(req->cmd->rw.slba) << req->ns->blksize_shift;
-
 	memset(&req->f.iocb, 0, sizeof(struct kiocb));
 	for_each_sg_page(req->sg, &sg_pg_iter, req->sg_cnt, 0) {
 		nvmet_file_init_bvec(&req->f.bvec[bv_cnt], &sg_pg_iter);
@@ -236,6 +240,12 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
 					sizeof(range));
 		if (ret)
 			break;
+
+		if (offset + len > req->ns->size) {
+			ret = NVME_SC_LBA_RANGE | NVME_SC_DNR;
+			break;
+		}
+
 		offset = le64_to_cpu(range.slba) << req->ns->blksize_shift;
 		len = le32_to_cpu(range.nlb) << req->ns->blksize_shift;
 		if (vfs_fallocate(req->ns->file, mode, offset, len)) {
@@ -283,6 +293,11 @@ static void nvmet_file_write_zeroes_work(struct work_struct *w)
 	len = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
 			req->ns->blksize_shift);
 
+	if (unlikely(offset + len > req->ns->size)) {
+		nvmet_req_complete(req, NVME_SC_LBA_RANGE | NVME_SC_DNR);
+		return;
+	}
+
 	ret = vfs_fallocate(req->ns->file, mode, offset, len);
 	nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
 }
-- 
2.14.1

[PATCH v2 for-4.18 1/2] nvmet: fix file discard return status

[Chaitanya Kulkarni]

Looks good.
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>




From: Sagi Grimberg <sagi@grimberg.me>
Sent: Wednesday, July 11, 2018 2:43 AM
To: linux-nvme at lists.infradead.org
Cc: Christoph Hellwig; Chaitanya Kulkarni
Subject: [PATCH v2 for-4.18 1/2] nvmet: fix file discard return status
? 
 
If nvmet_copy_from_sgl failed, we falsly return successful
completion status.

Fixes: d5eff33ee6f8 ("nvmet: add simple file backed ns support")
Signed-off-by: Sagi Grimberg <sagi at grimberg.me>
---
Changes from v1:
- make ret type u16

?drivers/nvme/target/io-cmd-file.c | 18 ++++++++++--------
?1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/nvme/target/io-cmd-file.c b/drivers/nvme/target/io-cmd-file.c
index 57c660e3245d..dad8d44bf90e 100644
--- a/drivers/nvme/target/io-cmd-file.c
+++ b/drivers/nvme/target/io-cmd-file.c
@@ -227,22 +227,24 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
?{
???????? int mode = FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE;
???????? struct nvme_dsm_range range;
-?????? loff_t offset;
-?????? loff_t len;
-?????? int i, ret;
+?????? loff_t offset, len;
+?????? u16 ret;
+?????? int i;
?
???????? for (i = 0; i <= le32_to_cpu(req->cmd->dsm.nr); i++) {
-?????????????? if (nvmet_copy_from_sgl(req, i * sizeof(range), &range,
-?????????????????????????????????????? sizeof(range)))
+?????????????? ret = nvmet_copy_from_sgl(req, i * sizeof(range), &range,
+?????????????????????????????????????? sizeof(range));
+?????????????? if (ret)
???????????????????????? break;
???????????????? offset = le64_to_cpu(range.slba) << req->ns->blksize_shift;
???????????????? len = le32_to_cpu(range.nlb) << req->ns->blksize_shift;
-?????????????? ret = vfs_fallocate(req->ns->file, mode, offset, len);
-?????????????? if (ret)
+?????????????? if (vfs_fallocate(req->ns->file, mode, offset, len)) {
+?????????????????????? ret = NVME_SC_INTERNAL | NVME_SC_DNR;
???????????????????????? break;
+?????????????? }
???????? }
?
-?????? nvmet_req_complete(req, ret < 0 ? NVME_SC_INTERNAL | NVME_SC_DNR : 0);
+?????? nvmet_req_complete(req, ret);
?}
?
?static void nvmet_file_dsm_work(struct work_struct *w)
-- 
2.14.1

    

[PATCH v2 for-4.18 1/2] nvmet: fix file discard return status

[Christoph Hellwig]

Thanks, applied.