diff mbox series

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

Message ID 20220628022325.14627-2-michael.christie@oracle.com
State New
Headers show
Series target: Unmap fixes | expand

Commit Message

Mike Christie June 28, 2022, 2:23 a.m. UTC
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(+)

Comments

Martin K. Petersen July 7, 2022, 8:42 p.m. UTC | #1
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.
Dmitry Bogdanov July 7, 2022, 8:57 p.m. UTC | #2
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.
Martin K. Petersen July 7, 2022, 9:38 p.m. UTC | #3
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!
diff mbox series

Patch

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.