On 7/27/21 6:39 PM, Martin K. Petersen wrote: > > Kees, > >> 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 <keescook@xxxxxxxxxxxx> > > 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@xxxxxxxxxxxxx> > > Acked-by: Martin K. Petersen <martin.petersen@xxxxxxxxxx> > >> --- >> 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); >
