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 <[email protected]>
---
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);
--
2.30.2
On 7/28/21 8:35 PM, Martin K. Petersen wrote:
>
> Kees,
>
>> For example, change it to:
>>
>> + BUILD_BUG_ON(sizeof(evt_struct->iu.srp) != SRP_MAX_IU_LEN);
>> + memset(&evt_struct->iu.srp, 0x00, sizeof(evt_struct->iu.srp));
>> srp_cmd = &evt_struct->iu.srp.cmd;
>> - memset(srp_cmd, 0x00, SRP_MAX_IU_LEN);
>
>> For the moment, I'll leave the patch as-is unless you prefer having
>> the BUILD_BUG_ON(). :)
>
> I'm OK with the BUILD_BUG_ON(). Hopefully Tyrel or Brian will chime in.
>
All the other srp structs are at most 64 bytes and the size of the union is
explicitly set to SRP_MAX_IU_LEN by the last field of the union.
union srp_iu {
struct srp_login_req login_req;
struct srp_login_rsp login_rsp;
struct srp_login_rej login_rej;
struct srp_i_logout i_logout;
struct srp_t_logout t_logout;
struct srp_tsk_mgmt tsk_mgmt;
struct srp_cmd cmd;
struct srp_rsp rsp;
u8 reserved[SRP_MAX_IU_LEN];
};
So, in my mind if SRP_MAX_IU_LEN ever changes so does the size of the union
making the BUILD_BUG_ON() superfluous. But it doesn't really hurt anything either.
-Tyrel