Skip to contentSkip to conversation

nvmet: check fileio lba range access boundaries

PATCH New Sagi Grimberg 3 participants 3 messages Jul 11, 2018 – Jul 12, 2018

Subsystems (2)
NVMe NVMe target
lore
Share & tools
lore

Sagi Grimberg

author maintainer
#0 lore raw

PATCH v3 2/2 nvmet: check fileio lba range access boundaries

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 v2:
- rebase accident - use offset + len after assigning them

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(-)
drivers/nvme/target/io-cmd-file.c +17 -2
index dad8d44bf90e..c2d0d08b59c8 100644
@@ -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,8 +240,14 @@ static void nvmet_file_execute_discard(struct nvmet_req *req)
 					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;
+		if (offset + len > req->ns->size) {
+			ret = NVME_SC_LBA_RANGE | NVME_SC_DNR;
+			break;
+		}
+
 		if (vfs_fallocate(req->ns->file, mode, offset, len)) {
 			ret = NVME_SC_INTERNAL | NVME_SC_DNR;
 			break;

@@ -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);
 }
Signature · 1 line
2.14.1
#1 lore raw

PATCH v3 2/2 nvmet: check fileio lba range access boundaries

Looks good to me.

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

From: Sagi Grimberg <[email protected]>
Sent: Wednesday, July 11, 2018 6:13 AM
To: linux-nvme at lists.infradead.org
Cc: Christoph Hellwig; Chaitanya Kulkarni
Subject: [PATCH v3 for-4.18 2/2] nvmet: check fileio lba range access boundaries ?

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 v2:
- rebase accident - use offset + len after assigning them

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(-)

drivers/nvme/target/io-cmd-file.c +0 -0
index dad8d44bf90e..c2d0d08b59c8 100644
@@ -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,8 +240,14 @@ static void nvmet_file_execute_discard(struct nvmet_req *req) ???????????????????????????????????????? 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; +?????????????? if (offset + len > req->ns->size) { +?????????????????????? ret = NVME_SC_LBA_RANGE | NVME_SC_DNR; +?????????????????????? break;
+?????????????? }
+
???????????????? if (vfs_fallocate(req->ns->file, mode, offset, len)) { ???????????????????????? ret = NVME_SC_INTERNAL | NVME_SC_DNR; ???????????????????????? break;
@@ -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); ?}

Signature · 1 line
2.14.1