| Subject | Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp | From | "Martin K. Petersen" <> | Date | Tue, 27 Jul 2021 21:39:39 -0400 |
| |
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@chromium.org>
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.
Acked-by: Martin K. Petersen <martin.petersen@oracle.com>
> --- > 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);
-- Martin K. Petersen Oracle Linux Engineering
|