| Message ID | 20220628022325.14627-2-michael.christie@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ccd3f449052449a917a3e577d8ba0368f43b8f29 |
| Headers | show |
| Series | target: Unmap fixes | expand |
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..)
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.
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.
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 --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.
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(+)