Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp
From: Tyrel Datwyler <tyreld@linux.ibm.com>
Date: 2021-07-30 18:16:55
Also in:
dri-devel, linux-block, linux-hardening, linux-kbuild, linux-wireless, lkml, netdev
Possibly related (same subject, not in this thread)
- 2021-07-30 · Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp · Tyrel Datwyler <tyreld@linux.ibm.com>
- 2021-07-29 · Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp · "Martin K. Petersen" <martin.petersen@oracle.com>
- 2021-07-27 · [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp · Kees Cook <hidden>
On 7/27/21 6:39 PM, Martin K. Petersen wrote:
Kees,quoted
In preparation for FORTIFY_SOURCE performing compile-time and run-time field bounds checking for memset(), avoid intentionally writing across neighboring fields. Instead of writing beyond the end of evt_struct->iu.srp.cmd, target the upper union (evt_struct->iu.srp) instead, as that's what is being wiped. Signed-off-by: Kees Cook <redacted>Orthogonal to your change, it wasn't immediately obvious to me that SRP_MAX_IU_LEN was the correct length to use for an srp_cmd. However, I traversed the nested unions and it does look OK. For good measure I copied Tyrel and Brian.
LGTM Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>quoted
--- drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c index e6a3eaaa57d9..7e8beb42d2d3 100644 --- a/drivers/scsi/ibmvscsi/ibmvscsi.c +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c@@ -1055,8 +1055,8 @@ static int ibmvscsi_queuecommand_lck(struct scsi_cmnd *cmnd, return SCSI_MLQUEUE_HOST_BUSY; /* Set up the actual SRP IU */ + memset(&evt_struct->iu.srp, 0x00, SRP_MAX_IU_LEN); srp_cmd = &evt_struct->iu.srp.cmd; - memset(srp_cmd, 0x00, SRP_MAX_IU_LEN); srp_cmd->opcode = SRP_CMD; memcpy(srp_cmd->cdb, cmnd->cmnd, sizeof(srp_cmd->cdb)); int_to_scsilun(lun, &srp_cmd->lun);