Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow by aiming at srp

Tyrel Datwyler
2021-07-30T19:11:27Z

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