| Message ID | ZHkseX6TiFahvxJA@work (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | a48e2c328c6505d356c90ef51a2052d1d27f9bef |
| Headers | show |
| Series | [v2,next] scsi: lpfc: Avoid -Wstringop-overflow warning | expand |
Thanks Gustavo and Kees. Reviewed-by: Justin Tee <justin.tee@broadcom.com> On Thu, Jun 1, 2023 at 4:43 PM 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 = 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 ‘lpfc_debugfs_ras_log_data’, > inlined from ‘lpfc_debugfs_ras_log_open’ at drivers/scsi/lpfc/lpfc_debugfs.c:2271:15: > drivers/scsi/lpfc/lpfc_debugfs.c:2210:25: warning: ‘memcpy’ specified bound between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] > 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/CABPRKS8zyzrbsWt4B5fp7kMowAZFiMLKg5kW26uELpg1cDKY3A@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_debugfs.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, struct file *file) > goto out; > } > spin_unlock_irq(&phba->hbalock); > - debug = kmalloc(sizeof(*debug), GFP_KERNEL); > + > + if (check_mul_overflow(LPFC_RAS_MIN_BUFF_POST_SIZE, > + phba->cfg_ras_fwlog_buffsize, &size)) > + goto out; > + > + debug = kzalloc(sizeof(*debug), GFP_KERNEL); > if (!debug) > goto out; > > - size = LPFC_RAS_MIN_BUFF_POST_SIZE * phba->cfg_ras_fwlog_buffsize; > debug->buffer = vmalloc(size); > if (!debug->buffer) > goto free_debug; > -- > 2.34.1 >
Gustavo, > Prevent any potential integer wrapping issue, and avoid a > -Wstringop-overflow warning by using the check_mul_overflow() helper. Applied to 6.5/scsi-staging, thanks!
On Thu, 01 Jun 2023 17:40:41 -0600, Gustavo A. R. Silva 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 = LPFC_RAS_MIN_BUFF_POST_SIZE * phba->cfg_ras_fwlog_buffsize; > > [...] Applied to 6.5/scsi-queue, thanks! [1/1] scsi: lpfc: Avoid -Wstringop-overflow warning https://git.kernel.org/mkp/scsi/c/a48e2c328c65
diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.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, struct file *file) goto out; } spin_unlock_irq(&phba->hbalock); - debug = kmalloc(sizeof(*debug), GFP_KERNEL); + + if (check_mul_overflow(LPFC_RAS_MIN_BUFF_POST_SIZE, + phba->cfg_ras_fwlog_buffsize, &size)) + goto out; + + debug = kzalloc(sizeof(*debug), GFP_KERNEL); if (!debug) goto out; - size = LPFC_RAS_MIN_BUFF_POST_SIZE * phba->cfg_ras_fwlog_buffsize; debug->buffer = vmalloc(size); if (!debug->buffer) goto free_debug;
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 = 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 ‘lpfc_debugfs_ras_log_data’, inlined from ‘lpfc_debugfs_ras_log_open’ at drivers/scsi/lpfc/lpfc_debugfs.c:2271:15: drivers/scsi/lpfc/lpfc_debugfs.c:2210:25: warning: ‘memcpy’ specified bound between 18446744071562067968 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=] 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/CABPRKS8zyzrbsWt4B5fp7kMowAZFiMLKg5kW26uELpg1cDKY3A@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(-)