[PATCH 0/1] target: Unmap fixes

[PATCH 0/1] target: Unmap fixes

[Mike Christie]

This patch made over Linus's tree fixes a crash hit when we get a
WRITE SAME with the NDOB bit set or if the initiator has sent a malformed
WRITE SAME that does not contain any data out buffer even though NDOB=0.

V2:
- This version of patch only fixes the crash. It does not implement
support. Plus other non fix patches put into a feature patchset.

[PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

[Mike Christie]

In newer version of the SBC specs, we have a NDOB bit that indicates there
is no data buffer that gets written out. If this bit is set using commands
like "sg_write_same --ndob" we will crash in target_core_iblock/file's
execute_write_same handlers when we go to access the se_cmd->t_data_sg
because its NULL.

This patch adds a check for the NDOB bit in the common WRITE SAME code
because we don't support it. And, it adds a check for zero SG elements in
each handler in case the initiator tries to send a normal WRITE SAME with
no data buffer.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/target/target_core_file.c   | 3 +++
 drivers/target/target_core_iblock.c | 4 ++++
 drivers/target/target_core_sbc.c    | 6 ++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/target/target_core_file.c b/drivers/target/target_core_file.c
index e68f1cc8ef98..6c8d8b051bfd 100644
--- a/drivers/target/target_core_file.c
+++ b/drivers/target/target_core_file.c
@@ -448,6 +448,9 @@ fd_execute_write_same(struct se_cmd *cmd)
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
 
+	if (!cmd->t_data_nents)
+		return TCM_INVALID_CDB_FIELD;
+
 	if (cmd->t_data_nents > 1 ||
 	    cmd->t_data_sg[0].length != cmd->se_dev->dev_attrib.block_size) {
 		pr_err("WRITE_SAME: Illegal SGL t_data_nents: %u length: %u"
diff --git a/drivers/target/target_core_iblock.c b/drivers/target/target_core_iblock.c
index 378c80313a0f..1ed9381751e6 100644
--- a/drivers/target/target_core_iblock.c
+++ b/drivers/target/target_core_iblock.c
@@ -494,6 +494,10 @@ iblock_execute_write_same(struct se_cmd *cmd)
 		       " backends not supported\n");
 		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 	}
+
+	if (!cmd->t_data_nents)
+		return TCM_INVALID_CDB_FIELD;
+
 	sg = &cmd->t_data_sg[0];
 
 	if (cmd->t_data_nents > 1 ||
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index ca1b2312d6e7..f6132836eb38 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -312,6 +312,12 @@ sbc_setup_write_same(struct se_cmd *cmd, unsigned char flags, struct sbc_ops *op
 		pr_warn("WRITE SAME with ANCHOR not supported\n");
 		return TCM_INVALID_CDB_FIELD;
 	}
+
+	if (flags & 0x01) {
+		pr_warn("WRITE SAME with NDOB not supported\n");
+		return TCM_INVALID_CDB_FIELD;
+	}
+
 	/*
 	 * Special case for WRITE_SAME w/ UNMAP=1 that ends up getting
 	 * translated into block discard requests within backend code.
-- 
2.25.1

Re: [PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

[Christoph Hellwig]

On Mon, Jun 27, 2022 at 09:23:25PM -0500, Mike Christie wrote:
> In newer version of the SBC specs, we have a NDOB bit that indicates there
> is no data buffer that gets written out. If this bit is set using commands
> like "sg_write_same --ndob" we will crash in target_core_iblock/file's
> execute_write_same handlers when we go to access the se_cmd->t_data_sg
> because its NULL.
> 
> This patch adds a check for the NDOB bit in the common WRITE SAME code
> because we don't support it. And, it adds a check for zero SG elements in
> each handler in case the initiator tries to send a normal WRITE SAME with
> no data buffer.

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

(and I wonder if we have similar problems with other commands, the
target code could use same targeted fuzzing..)

Re: [PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

[Martin K. Petersen]

Christoph,

> (and I wonder if we have similar problems with other commands, the
> target code could use same targeted fuzzing..)

Yeah.

The USB gadget series implements initial support for RSOC. It might be
worth entertaining to augment that code with CDB masks for all the
commands we actually support. And then leverage these masks for command
validation.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

[Dmitry Bogdanov]

Hi Martin,

On Thu, Jul 07, 2022 at 04:42:36PM -0400, Martin K. Petersen wrote:
> 
> Christoph,
> 
> > (and I wonder if we have similar problems with other commands, the
> > target code could use same targeted fuzzing..)
> 
> Yeah.
> 
> The USB gadget series implements initial support for RSOC. It might be
> worth entertaining to augment that code with CDB masks for all the
> commands we actually support. And then leverage these masks for command
> validation.

I have a patchset with complete RSOC support with CDB mask. Even dynamic
mask due to device configuration. It passes all RSOC related libiscsi
tests.
If the community want it I may send it.

Re: [PATCH 1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash

[Martin K. Petersen]

Dmitry,

> I have a patchset with complete RSOC support with CDB mask. Even
> dynamic mask due to device configuration. It passes all RSOC related
> libiscsi tests.  If the community want it I may send it.

That would be great, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 0/1] target: Unmap fixes

[Martin K. Petersen]

On Mon, 27 Jun 2022 21:23:24 -0500, Mike Christie wrote:

> This patch made over Linus's tree fixes a crash hit when we get a
> WRITE SAME with the NDOB bit set or if the initiator has sent a malformed
> WRITE SAME that does not contain any data out buffer even though NDOB=0.
> 
> V2:
> - This version of patch only fixes the crash. It does not implement
> support. Plus other non fix patches put into a feature patchset.
> 
> [...]

Applied to 5.19/scsi-fixes, thanks!

[1/1] scsi: target: Fix WRITE_SAME No Data Buffer crash
      https://git.kernel.org/mkp/scsi/c/ccd3f4490524

-- 
Martin K. Petersen	Oracle Linux Engineering