Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <ZHkseX6TiFahvxJA@work>
In-Reply-To: <ZHkseX6TiFahvxJA@work>
From:   Justin Tee <justintee8345@gmail.com>
Date:   Thu, 1 Jun 2023 18:10:36 -0700
Message-ID: <CABPRKS_FD=oeJGAEk2kpiwxSP-eDRXmm0iMhDfOW0CLV4qcS3Q@mail.gmail.com>
Subject: Re: [PATCH v2][next] scsi: lpfc: Avoid -Wstringop-overflow warning
To:     "Gustavo A. R. Silva" <gustavoars@kernel.org>,
        Kees Cook <keescook@chromium.org>
Cc:     James Smart <james.smart@broadcom.com>,
        Dick Kennedy <dick.kennedy@broadcom.com>,
        Justin Tee <justin.tee@broadcom.com>,
        "James E.J. Bottomley" <jejb@linux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>,
        linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Thanks Gustavo and Kees.

Reviewed-by: Justin Tee <justin.tee@broadcom.com>

On Thu, Jun 1, 2023 at 4:43=E2=80=AFPM Gustavo A. R. Silva
<gustavoars@kernel.org> wrote:
>
> Prevent any potential integer wrapping issue, and avoid a
> -Wstringop-overflow warning by using the check_mul_overflow() helper.
>
> drivers/scsi/lpfc/lpfc.h:
> 837:#define LPFC_RAS_MIN_BUFF_POST_SIZE (256 * 1024)
>
> drivers/scsi/lpfc/lpfc_debugfs.c:
> 2266 size =3D LPFC_RAS_MIN_BUFF_POST_SIZE * phba->cfg_ras_fwlog_buffsize;
>
> this can wrap to negative if cfg_ras_fwlog_buffsize is large
> enough. And even when in practice this is not possible (due to
> phba->cfg_ras_fwlog_buffsize never being larger than 4[1]), the
> compiler is legitimately warning us about potentially buggy code.
>
> Fix the following warning seen under GCC-13:
> In function =E2=80=98lpfc_debugfs_ras_log_data=E2=80=99,
>     inlined from =E2=80=98lpfc_debugfs_ras_log_open=E2=80=99 at drivers/s=
csi/lpfc/lpfc_debugfs.c:2271:15:
> drivers/scsi/lpfc/lpfc_debugfs.c:2210:25: warning: =E2=80=98memcpy=E2=80=
=99 specified bound between 18446744071562067968 and 18446744073709551615 e=
xceeds maximum object size 9223372036854775807 [-Wstringop-overflow=3D]
>  2210 |                         memcpy(buffer + copied, dmabuf->virt,
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  2211 |                                size - copied - 1);
>       |                                ~~~~~~~~~~~~~~~~~~
>
> Link: https://github.com/KSPP/linux/issues/305
> Link: https://lore.kernel.org/linux-hardening/CABPRKS8zyzrbsWt4B5fp7kMowA=
ZFiMLKg5kW26uELpg1cDKY3A@mail.gmail.com/ [1]
> Co-developed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---
> Changes in v2:
>  - Use check_mul_overflow() helper (Kees).
>
> v1:
>  - Link: https://lore.kernel.org/linux-hardening/ZHZq7AV9Q2WG1xRB@work/
>
>  drivers/scsi/lpfc/lpfc_debugfs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_de=
bugfs.c
> index bdf34af4ef36..7f9b221e7c34 100644
> --- a/drivers/scsi/lpfc/lpfc_debugfs.c
> +++ b/drivers/scsi/lpfc/lpfc_debugfs.c
> @@ -2259,11 +2259,15 @@ lpfc_debugfs_ras_log_open(struct inode *inode, st=
ruct file *file)
>                 goto out;
>         }
>         spin_unlock_irq(&phba->hbalock);
> -       debug =3D kmalloc(sizeof(*debug), GFP_KERNEL);
> +
> +       if (check_mul_overflow(LPFC_RAS_MIN_BUFF_POST_SIZE,
> +                              phba->cfg_ras_fwlog_buffsize, &size))
> +               goto out;
> +
> +       debug =3D kzalloc(sizeof(*debug), GFP_KERNEL);
>         if (!debug)
>                 goto out;
>
> -       size =3D LPFC_RAS_MIN_BUFF_POST_SIZE * phba->cfg_ras_fwlog_buffsi=
ze;
>         debug->buffer =3D vmalloc(size);
>         if (!debug->buffer)
>                 goto free_debug;
> --
> 2.34.1
>