Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset() overflow
 by aiming at srp
To:     "Martin K. Petersen" <martin.petersen@oracle.com>,
        Kees Cook <keescook@chromium.org>
Cc:     linux-hardening@vger.kernel.org,
        "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Keith Packard <keithpac@amazon.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, linux-wireless@vger.kernel.org,
        netdev@vger.kernel.org, dri-devel@lists.freedesktop.org,
        linux-staging@lists.linux.dev, linux-block@vger.kernel.org,
        linux-kbuild@vger.kernel.org, clang-built-linux@googlegroups.com,
        Brian King <brking@linux.vnet.ibm.com>
References: <20210727205855.411487-1-keescook@chromium.org>
 <20210727205855.411487-37-keescook@chromium.org>
 <yq135rzp79c.fsf@ca-mkp.ca.oracle.com> <202107281152.515A3BA@keescook>
 <yq1k0l9oktw.fsf@ca-mkp.ca.oracle.com>
From:   Tyrel Datwyler <tyreld@linux.ibm.com>
Message-ID: <3ffbcf75-166e-5802-1d8e-9c7739961b80@linux.ibm.com>
Date:   Fri, 30 Jul 2021 12:11:02 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.11.0
MIME-Version: 1.0
In-Reply-To: <yq1k0l9oktw.fsf@ca-mkp.ca.oracle.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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