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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <yq135rzp79c.fsf@ca-mkp.ca.oracle.com>
Date:   Tue, 27 Jul 2021 21:39:39 -0400
From:   "Martin K. Petersen" <martin.petersen@...cle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     linux-hardening@...r.kernel.org,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        Keith Packard <keithpac@...zon.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-staging@...ts.linux.dev, linux-block@...r.kernel.org,
        linux-kbuild@...r.kernel.org, clang-built-linux@...glegroups.com,
        Brian King <brking@...ux.vnet.ibm.com>,
        Tyrel Datwyler <tyreld@...ux.ibm.com>
Subject: Re: [PATCH 36/64] scsi: ibmvscsi: Avoid multi-field memset()
 overflow by aiming at srp


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@...omium.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@...cle.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