Skip to contentSkip to conversation

scsi: be2iscsi: validate vendor command buffers

RFC PATCH New BeaCox 1 participant 3 messages Nov 1

Subsystems (3)
lore
Share & tools
lore

BeaCox

author
#0 lore raw

cover scsi: be2iscsi: validate vendor command buffers

These patches validate the vendor request and DMA payload before they are accessed, then bound the WRITE_FLASH copy by the remaining buffer space. The first patch also keeps the allocation size valid for the caller's error cleanup. The second preserves the existing u16 offset interpretation and scatterlist copy limit.

The series is based on mainline cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a. Both the first patch and the complete series were object-built on x86_64 with gcc 13.3 and W=1. An ASan/UBSan userspace harness extracting the actual function detects out-of-bounds accesses in the original code and passes 2593 boundary cases after both patches.

There is no kernel/BSG reproducer or HBA/firmware test here. The BSG entry point requires CAP_SYS_RAWIO; no unprivileged exploit is claimed. The local harness can be shared privately on request. Feedback and hardware testing are welcome, especially on valid vendor commands.

The analysis, patches and validation were assisted by OpenAI Codex.

BeaCox (2):
scsi: be2iscsi: validate vendor request and payload lengths
scsi: be2iscsi: bound the WRITE_FLASH copy to the DMA buffer

 drivers/scsi/be2iscsi/be_mgmt.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

base-commit: cbbddb6b08cc9900a87cccaa9b22bbe06aa4f50a

Signature · 1 line
2.43.0
author
#1 lore raw

RFC PATCH 1/2 scsi: be2iscsi: validate vendor request and payload lengths

mgmt_vendor_specific_fw_cmd() reads vendor_cmd[0..4], but the iSCSI BSG transport accepts vendor requests containing only msgcode and vendor_id. Reject requests shorter than the five command words before accessing those fields.

The DMA buffer is allocated using request_payload.payload_len. Require space for be_bsg_vendor_cmd before writing its fields or command header.

Keep nonemb_cmd->size initialized before returning an error because beiscsi_bsg_request() uses it to free the DMA allocation. Return zero, the failure sentinel expected by that caller.

The analysis, draft and validation were assisted by OpenAI Codex.

Fixes ffce3e2e8ce4 ("[SCSI] be2iscsi: Adding bsg interface for be2iscsi")

---
Testing: x86_64 object builds with gcc 13.3 and W=1 for the first patch and the complete series. A userspace harness extracts the actual driver function and uses hardware stubs with ASan/UBSan. It detects out-of-bounds accesses in the original function and passes 2593 boundary cases with both patches applied, including error cleanup and lock balance.

This is function-level testing, not a kernel/BSG reproducer or HBA test. No be2iscsi HBA or firmware operation was exercised. The BSG entry point requires CAP_SYS_RAWIO; no unprivileged exploit is claimed. The local harness can be shared privately on request.

 drivers/scsi/be2iscsi/be_mgmt.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/scsi/be2iscsi/be_mgmt.c +9 -0
index b1cba986f0..3b8551a1f0 100644
@@ -43,6 +43,15 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 	unsigned short region, sector_size, sector, offset;
 
 	nonemb_cmd->size = job->request_payload.payload_len;
+	if (job->request_len <
+	    offsetof(struct iscsi_bsg_request,
+		     rqst_data.h_vendor.vendor_cmd) + 5 * sizeof(__u32) ||
+	    nonemb_cmd->size < sizeof(*req)) {
+		beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
+			    "BG_%d : Invalid vendor request or payload length\n");
+		return 0;
+	}
+
 	memset(nonemb_cmd->va, 0, nonemb_cmd->size);
 	region =  bsg_req->rqst_data.h_vendor.vendor_cmd[1];
 	sector_size =  bsg_req->rqst_data.h_vendor.vendor_cmd[2];
Signature · 1 line
2.43.0
author
#2 lore raw

RFC PATCH 2/2 scsi: be2iscsi: bound the WRITE_FLASH copy to the DMA buffer

In reply to BeaCox

The WRITE_FLASH command derives an offset from vendor_cmd[2..4] and copies the request payload to nonemb_cmd->va + offset. The allocation contains only request_payload.payload_len bytes, so a nonzero offset can place the copy beyond the allocation.

Check the offset and remaining space before forming the destination. Bound the copy length by both request_len and the payload length, matching the existing scatterlist copy limit. This keeps requests with request_len larger than payload_len valid when the actual copy fits.

Keep the existing u16 offset interpretation; this change bounds the memory access rather than changing the firmware command format.

The analysis, draft and validation were assisted by OpenAI Codex.

Fixes ffce3e2e8ce4 ("[SCSI] be2iscsi: Adding bsg interface for be2iscsi")
---
 drivers/scsi/be2iscsi/be_mgmt.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/scsi/be2iscsi/be_mgmt.c +10 -1
index 3b8551a1f0..1b95161cf0 100644
@@ -41,6 +41,7 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 	struct iscsi_bsg_request *bsg_req = job->request;
 	struct be_bsg_vendor_cmd *req = nonemb_cmd->va;
 	unsigned short region, sector_size, sector, offset;
+	unsigned int len;
 
 	nonemb_cmd->size = job->request_payload.payload_len;
 	if (job->request_len <
@@ -66,11 +67,19 @@ unsigned int mgmt_vendor_specific_fw_cmd(struct be_ctrl_info *ctrl,
 	switch (bsg_req->rqst_data.h_vendor.vendor_cmd[0]) {
 	case BEISCSI_WRITE_FLASH:
 		offset = sector * sector_size + offset;
+		len = min(job->request_len, nonemb_cmd->size);
+		if (offset > nonemb_cmd->size ||
+		    len > nonemb_cmd->size - offset) {
+			beiscsi_log(phba, KERN_WARNING, BEISCSI_LOG_CONFIG,
+				    "BG_%d : WRITE_FLASH copy exceeds DMA buffer\n");
+			mutex_unlock(&ctrl->mbox_lock);
+			return 0;
+		}
 		be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
 				   OPCODE_COMMON_WRITE_FLASH, sizeof(*req));
 		sg_copy_to_buffer(job->request_payload.sg_list,
 				  job->request_payload.sg_cnt,
-				  nonemb_cmd->va + offset, job->request_len);
+				  nonemb_cmd->va + offset, len);
 		break;
 	case BEISCSI_READ_FLASH:
 		be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_ISCSI,
Signature · 1 line
2.43.0