enabled -Wformat-truncation for clang
Subsystems (17)
cover enabled -Wformat-truncation for clang
From: Arnd Bergmann <[email protected]>
With randconfig build testing, I found only eight files that produce warnings with clang when -Wformat-truncation is enabled. This means we can just turn it on by default rather than only enabling it for "make W=1".
Unfortunately, gcc produces a lot more warnings when the option is enabled, so it's not yet possible to turn it on both both compilers.
I hope that the patches can get picked up by platform maintainers directly, so the final patch can go in later on.
Arnd
Arnd Bergmann (9):
fbdev: shmobile: fix snprintf truncation
enetc: avoid truncating error message
qed: avoid truncating work queue length
mlx5: avoid truncating error message
surface3_power: avoid format string truncation warning
Input: IMS: fix printf string overflow
scsi: mylex: fix sysfs buffer lengths
ALSA: aoa: avoid false-positive format truncation warning
kbuild: enable -Wformat-truncation on clang
drivers/input/misc/ims-pcu.c | 4 ++-- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- .../ethernet/mellanox/mlx5/core/esw/bridge.c | 2 +- drivers/net/ethernet/qlogic/qed/qed_main.c | 9 ++++--- drivers/platform/surface/surface3_power.c | 2 +- drivers/scsi/myrb.c | 20 ++++++++-------- drivers/scsi/myrs.c | 24 +++++++++---------- drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +- scripts/Makefile.extrawarn | 2 ++ sound/aoa/soundbus/i2sbus/core.c | 2 +- 10 files changed, 35 insertions(+), 34 deletions(-)
Signature · 38 lines
2.39.2 Cc: Dmitry Torokhov <[email protected]> Cc: Claudiu Manoil <[email protected]> Cc: Vladimir Oltean <[email protected]> Cc: Jakub Kicinski <[email protected]> Cc: Saeed Mahameed <[email protected]> Cc: Leon Romanovsky <[email protected]> Cc: Ariel Elior <[email protected]> Cc: Manish Chopra <[email protected]> Cc: Hans de Goede <[email protected]> Cc: "Ilpo Järvinen" <[email protected]> Cc: Maximilian Luz <[email protected]> Cc: Hannes Reinecke <[email protected]> Cc: "Martin K. Petersen" <[email protected]> Cc: Helge Deller <[email protected]> Cc: Masahiro Yamada <[email protected]> Cc: Nathan Chancellor <[email protected]> Cc: Nicolas Schier <[email protected]> Cc: Johannes Berg <[email protected]> Cc: Jaroslav Kysela <[email protected]> Cc: Takashi Iwai <[email protected]> Cc: Nick Desaulniers <[email protected]> Cc: Bill Wendling <[email protected]> Cc: Justin Stitt <[email protected]> Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected] Cc: [email protected]
PATCH 1/9 fbdev: shmobile: fix snprintf truncation
From: Arnd Bergmann <[email protected]>
The name of the overlay does not fit into the fixed-length field:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1577:2: error: 'snprintf' will always be truncated; specified size is 16, but format string expands to at least 25
Make it short enough by changing the string.
--- drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/video/fbdev/sh_mobile_lcdcfb.c +1 -1
@@ -1575,7 +1575,7 @@ sh_mobile_lcdc_overlay_fb_init(struct sh_mobile_lcdc_overlay *ovl)*/ info->fix = sh_mobile_lcdc_overlay_fix;snprintf (info->fix.id,sizeof (info->fix.id), -"SH Mobile LCDC Overlay %u" , ovl->index); +"SHMobile ovl %u" , ovl->index); info->fix.smem_start = ovl->dma_handle; info->fix.smem_len = ovl->fb_size; info->fix.line_length = ovl->pitch;
Signature · 1 line
2.39.2
Re: [PATCH 1/9] fbdev: shmobile: fix snprintf truncation
Hi Arnd,
Thank you for the patch.
Quoted 10 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
The name of the overlay does not fit into the fixed-length field:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1577:2: error: 'snprintf' will always be truncated; specified size is 16, but format string expands to at least 25Make it short enough by changing the string.
Fixes c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")Signed-off-by Arnd Bergmann <[email protected]>
Quoted 17 lines
--- drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/video/fbdev/sh_mobile_lcdcfb.c +1
-1@@ -1575,7 +1575,7 @@ sh_mobile_lcdc_overlay_fb_init(struct sh_mobile_lcdc_overlay *ovl)*/ info->fix = sh_mobile_lcdc_overlay_fix;snprintf (info->fix.id,sizeof (info->fix.id), -"SH Mobile LCDC Overlay %u" , ovl->index); +"SHMobile ovl %u" , ovl->index); info->fix.smem_start = ovl->dma_handle; info->fix.smem_len = ovl->fb_size; info->fix.line_length = ovl->pitch;
Signature · 3 lines
Regards, Laurent Pinchart
Re: [PATCH 1/9] fbdev: shmobile: fix snprintf truncation
In reply to Arnd BergmannQuoted 10 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
The name of the overlay does not fit into the fixed-length field:
drivers/video/fbdev/sh_mobile_lcdcfb.c:1577:2: error: 'snprintf' will always be truncated; specified size is 16, but format string expands to at least 25Make it short enough by changing the string.
Fixes c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")Signed-off-by Arnd Bergmann <[email protected]>
applied to fbdev git tree.
Thanks!
Helge
Quoted 17 lines
--- drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/video/fbdev/sh_mobile_lcdcfb.c +1
-1@@ -1575,7 +1575,7 @@ sh_mobile_lcdc_overlay_fb_init(struct sh_mobile_lcdc_overlay *ovl)*/ info->fix = sh_mobile_lcdc_overlay_fix;snprintf (info->fix.id,sizeof (info->fix.id), -"SH Mobile LCDC Overlay %u" , ovl->index); +"SHMobile ovl %u" , ovl->index); info->fix.smem_start = ovl->dma_handle; info->fix.smem_len = ovl->fb_size; info->fix.line_length = ovl->pitch;
PATCH 2/9 enetc: avoid truncating error message
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
As clang points out, the error message in enetc_setup_xdp_prog() still does not fit in the buffer and will be truncated:
drivers/net/ethernet/freescale/enetc/enetc.c:2771:3: error: 'snprintf' will always be truncated; specified size is 80, but format string expands to at least 87 [-Werror,-Wformat-truncation]
Replace it with an even shorter message that should fit.
--- drivers/net/ethernet/freescale/enetc/enetc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/freescale/enetc/enetc.c +1 -1
@@ -2769,7 +2769,7 @@ static int enetc_setup_xdp_prog(struct net_device *ndev, struct bpf_prog *prog,if (priv->min_num_stack_tx_queues + num_xdp_tx_queues > priv->num_tx_rings) {NL_SET_ERR_MSG_FMT_MOD (extack, -"Reserving %d XDP TXQs does not leave a minimum of %d for stack (total %d)" , +"Reserving %d XDP TXQs leaves under %d for stack (total %d)" , num_xdp_tx_queues, priv->min_num_stack_tx_queues, priv->num_tx_rings);
Signature · 1 line
2.39.2
PATCH 3/9 qed: avoid truncating work queue length
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang complains that the temporary string for the name passed into alloc_workqueue() is too short for its contents:
drivers/net/ethernet/qlogic/qed/qed_main.c:1218:3: error: 'snprintf' will always be truncated; specified size is 16, but format string expands to at least 18 [-Werror,-Wformat-truncation]
There is no need for a temporary buffer, and the actual name of a workqueue is 32 bytes (WQ_NAME_LEN), so just use the interface as intended to avoid the truncation.
--- drivers/net/ethernet/qlogic/qed/qed_main.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/net/ethernet/qlogic/qed/qed_main.c +4 -5
@@ -1206,7 +1206,6 @@ static void qed_slowpath_task(struct work_struct *work)static int qed_slowpath_wq_start (struct qed_dev *cdev) {struct qed_hwfn *hwfn; -char name[NAME_SIZE ];int i;if (IS_VF (cdev))
@@ -1215,11 +1214,11 @@ static int qed_slowpath_wq_start(struct qed_dev *cdev)for_each_hwfn (cdev, i) { hwfn = &cdev->hwfns[i]; -snprintf (name,NAME_SIZE ,"slowpath-%02x:%02x.%02x" , - cdev->pdev->bus->number, -PCI_SLOT (cdev->pdev->devfn), hwfn->abs_pf_id); + hwfn->slowpath_wq =alloc_workqueue ("slowpath-%02x:%02x.%02x" , +0 ,0 , cdev->pdev->bus->number, +PCI_SLOT (cdev->pdev->devfn), + hwfn->abs_pf_id); - hwfn->slowpath_wq =alloc_workqueue (name,0 ,0 );if (!hwfn->slowpath_wq) {DP_NOTICE (hwfn,"Cannot create slowpath workqueue\n" );return -ENOMEM ;
Signature · 1 line
2.39.2
RE: [EXTERNAL] [PATCH 3/9] qed: avoid truncating work queue length
Hi,
Quoted 51 lines
-----Original Message-----
From: Arnd Bergmann <[email protected]>
Sent: Wednesday, March 27, 2024 4:08 AM
To: [email protected]; Ariel Elior <[email protected]>; Manish Chopra <[email protected]>
Cc: Arnd Bergmann <[email protected]>; David S. Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Nathan Chancellor <[email protected]>; Nick Desaulniers <[email protected]>; Bill Wendling <[email protected]>; Justin Stitt <[email protected]>; Simon Horman <[email protected]>; Konstantin Khorenko <[email protected]>; Sudarsana Reddy Kalluru <[email protected]>; [email protected]; linux- [email protected]
Subject: [PATCH 3/9] qed: avoid truncating work queue lengthFrom: Arnd Bergmann <[email protected]>
clang complains that the temporary string for the name passed into
alloc_workqueue()is too short for its contents:
drivers/net/ethernet/qlogic/qed/qed_main.c:1218:3: error: 'snprintf' will always be truncated; specified size is 16, but format string expands to at least 18 [- Werror,-Wformat-truncation]There is no need for a temporary buffer, and the actual name of a workqueue is 32 bytes (
WQ_NAME_LEN), so just use the interface as intended to avoid the truncation.Fixes 59ccf86fe69a ("qed: Add driver infrastucture for handling mfw requests.")Signed-off-by Arnd Bergmann <[email protected]>---
drivers/net/ethernet/qlogic/qed/qed_main.c| 9 ++++-----1 file changed, 4 insertions(+), 5 deletions(-)drivers/net/ethernet/qlogic/qed/qed_main.c +0
-1@@ -1206,7 +1206,6 @@ static void qed_slowpath_task(struct work_struct *work)static int qed_slowpath_wq_start (struct qed_dev *cdev) {struct qed_hwfn *hwfn; -char name[NAME_SIZE ];int i;if (IS_VF(cdev))drivers/net/ethernet/qlogic/qed/qed_main.c +0
-0@@ -1215,11 +1214,11 @@ static int qed_slowpath_wq_start(struct qed_dev *cdev)for_each_hwfn (cdev, i) {
drivers/net/ethernet/qlogic/qed/qed_main.c static int qed_slowpath_wq_start(struct qed_dev · line 1222
Quoted 10 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +4
-3hwfn = &cdev->hwfns[i]; -snprintf (name,NAME_SIZE ,"slowpath-%02x:%02x.%02x" , - cdev->pdev->bus->number, -PCI_SLOT (cdev->pdev->devfn), hwfn->abs_pf_id); + hwfn->slowpath_wq = alloc_workqueue("slowpath- %02x:%02x.%02x", +0 ,0 , cdev->pdev->bus->number, +PCI_SLOT (cdev->pdev->devfn), + hwfn->abs_pf_id);
Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x", cdev->pdev->bus->number, PCI_SLOT(cdev->pdev->devfn), hwfn->abs_pf_id, 0, 0); Right?
Thanks,
Sundeep
Quoted 9 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +0
-1- hwfn->slowpath_wq =alloc_workqueue (name,0 ,0 );if (!hwfn->slowpath_wq) { DP_NOTICE(hwfn, "Cannot create slowpathworkqueue\n"); return -ENOMEM;--
2.39.2
Re: [EXTERNAL] [PATCH 3/9] qed: avoid truncating work queue length
Quoted 5 lines · Subbaraya Sundeep Bhatta
Quoted 5 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +1
-3-snprintf (name,NAME_SIZE ,"slowpath-%02x:%02x.%02x" , - cdev->pdev->bus->number, -PCI_SLOT (cdev->pdev->devfn), hwfn->abs_pf_id); + hwfn->slowpath_wq = alloc_workqueue("slowpath-%02x:%02x.%02x",
Quoted 8 lines
drivers/net/ethernet/qlogic/qed/qed_main.c static int qed_slowpath_wq_start(struct qed_dev *cdev) · line 1220
Quoted 3 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +3
-0+0 ,0 , cdev->pdev->bus->number, +PCI_SLOT (cdev->pdev->devfn), + hwfn->abs_pf_id);Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x",
cdev->pdev->bus->number,PCI_SLOT(cdev->pdev->devfn),hwfn->abs_pf_id, 0, 0);
Right?
I still think my version is the right one here, see the prototype:
__printf(1, 4) struct workqueue_struct *alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...);
so the first argument in the format, while the printf arguments start after the flags and max_active arguments that are still both set to zero.
Arnd
RE: [EXTERNAL] [PATCH 3/9] qed: avoid truncating work queue length
Hi,
Quoted 35 lines
-----Original Message-----
From: Arnd Bergmann <[email protected]>
Sent: Wednesday, March 27, 2024 9:05 PM
To: Subbaraya Sundeep Bhatta <[email protected]>; Arnd Bergmann <[email protected]>; [email protected]; Ariel Elior <[email protected]>; Manish Chopra <[email protected]>
Cc: David S . Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Nathan Chancellor <[email protected]>; Nick Desaulniers <[email protected]>; Bill Wendling <[email protected]>; Justin Stitt <[email protected]>; Simon Horman <[email protected]>; Konstantin Khorenko <[email protected]>; Sudarsana Reddy Kalluru <[email protected]>; Netdev <[email protected]>; linux- [email protected]
Subject: Re: [EXTERNAL] [PATCH 3/9] qed: avoid truncating work queue lengthQuoted 5 lines · Subbaraya Sundeep Bhatta
Quoted 5 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +1
-3-snprintf (name,NAME_SIZE ,"slowpath-%02x:%02x.%02x" , - cdev->pdev->bus->number, -PCI_SLOT (cdev->pdev->devfn), hwfn->abs_pf_id); + hwfn->slowpath_wq = alloc_workqueue("slowpath-%02x:%02x.%02x",
Quoted 8 lines
drivers/net/ethernet/qlogic/qed/qed_main.c static int qed_slowpath_wq_start(struct qed_dev *cdev) · line 1220
Quoted 3 lines
drivers/net/ethernet/qlogic/qed/qed_main.c +3
-0+0 ,0 , cdev->pdev->bus->number, +PCI_SLOT (cdev->pdev->devfn), + hwfn->abs_pf_id);Confused. This should be alloc_workqueue("slowpath-%02x:%02x.%02x",
cdev->pdev->bus->number,PCI_SLOT(cdev->pdev->devfn),hwfn->abs_pf_id, 0, 0);
Right?I still think my version is the right one here, see the prototype:
Quoted 7 lines
__printf(1, 4)structworkqueue_struct*alloc_workqueue(const char *fmt, unsigned int flags, int max_active, ...);so the first argument in the format, while the printf arguments start after the flags and
max_activearguments that are still both set to zero.
My bad. Got it
Thanks,
Sundeep
Quoted 1 line
Arnd
PATCH 4/9 mlx5: avoid truncating error message
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang warns that one error message is too long for its destination buffer:
drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c:1876:4: error: 'snprintf' will always be truncated; specified size is 80, but format string expands to at least 94 [-Werror,-Wformat-truncation-non-kprintf]
Reword it to be a bit shorter so it always fits.
--- drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c +1 -1
@@ -1874,7 +1874,7 @@ int mlx5_esw_bridge_port_mdb_add(struct net_device *dev, u16 vport_num, u16 esw_"Failed to lookup bridge port vlan metadata to create MDB (MAC=%pM,vid=%u,vport=%u)\n" , addr, vid, vport_num);NL_SET_ERR_MSG_FMT_MOD (extack, -"Failed to lookup bridge port vlan metadata to create MDB (MAC=%pM,vid=%u,vport=%u)\n" , +"Failed to lookup vlan metadata for MDB (MAC=%pM,vid=%u,vport=%u)\n" , addr, vid, vport_num);return -EINVAL ; }
Signature · 1 line
2.39.2
RE: [EXTERNAL] [PATCH 4/9] mlx5: avoid truncating error message
Hi,
Quoted 19 lines
-----Original Message-----
From: Arnd Bergmann <[email protected]>
Sent: Wednesday, March 27, 2024 4:08 AM
To: [email protected]; Saeed Mahameed <[email protected]>; Leon Romanovsky <[email protected]>
Cc: Arnd Bergmann <[email protected]>; David S. Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Nathan Chancellor <[email protected]>; Nick Desaulniers <[email protected]>; Bill Wendling <[email protected]>; Justin Stitt <[email protected]>; Vlad Buslov <[email protected]>; Roi Dayan <[email protected]>; Maor Dickman <[email protected]>; Gal Pressman <[email protected]>; [email protected]; [email protected]; linux- [email protected]
Subject: [PATCH 4/9] mlx5: avoid truncating error messageFrom: Arnd Bergmann <[email protected]>
clang warns that one error message is too long for its destination buffer:
Quoted 8 lines
drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c:1876:4: error: 'snprintf' will always be truncated; specified size is 80, but format string expands to at least 94 [-Werror,-Wformat-truncation-non-kprintf]Reword it to be a bit shorter so it always fits.
Fixes 70f0302b3f20 ("net/mlx5: Bridge, implement mdb offload")Signed-off-by Arnd Bergmann <[email protected]>
Thanks,
Sundeep
Quoted 25 lines
---
drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c| 2 +-1 file changed, 1 insertion(+), 1 deletion(-)drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c +1
-1@@ -1874,7 +1874,7 @@ int mlx5_esw_bridge_port_mdb_add(struct net_device *dev, u16 vport_num, u16 esw_ "Failed to lookup bridge port vlan metadata to create MDB (MAC=%pM,vid=%u,vport=%u)\n", addr, vid, vport_num);NL_SET_ERR_MSG_FMT_MOD (extack, - "Failed to lookup bridge port vlan metadata to create MDB (MAC=%pM,vid=%u,vport=%u)\n", + "Failed to lookup vlan metadata forMDB (MAC=%pM,vid=%u,vport=%u)\n",
addr, vid, vport_num); return -EINVAL; }--
2.39.2
PATCH 5/9 surface3_power: avoid format string truncation warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang warns about printing a pair of escaped strings into a buffer that is too short:
drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]
248 | snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf);
| ^Change the format string two print two less bytes so it always fits. The string is still truncated, so there is no change in behavior, but the compiler no longer warns about it.
---
Not entirely sure about this one, as I've never used escaped strings, and don't know if gcc is correct to warn here, or if the kernel defines it differently from the standard.
--- drivers/platform/surface/surface3_power.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/platform/surface/surface3_power.c +1 -1
@@ -245,7 +245,7 @@ static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)dev_err (&client->dev,"Error reading serial no: %d\n" , ret);return ret; }else { -snprintf (bix->serial,ARRAY_SIZE (bix->serial),"%3pE%6pE" , buf +7 , buf); +snprintf (bix->serial,ARRAY_SIZE (bix->serial),"%3pE%4pE" , buf +7 , buf); }/* get cycle count */
Signature · 1 line
2.39.2
Nathan Chancellor
maintainerRe: [PATCH 5/9] surface3_power: avoid format string truncation warning
Quoted 13 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns about printing a pair of escaped strings into a buffer that is too short:
drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]248 | snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf); | ^Change the format string two print two less bytes so it always fits. The string is still truncated, so there is no change in behavior, but the compiler no longer warns about it.
Quoted 6 lines
Fixes 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")Signed-off-by Arnd Bergmann <[email protected]>---
Not entirely sure about this one, as I've never used escaped strings, and don't know if gcc is correct to warn here, or if the kernel defines it differently from the standard.
As far as I understand it, this is a false positive because clang does not understand the kernel's %p extensions. GCC does not warn for overflow or truncation when %p is involved but the clang developers chose to intentionally deviate from GCC in that aspect while sticking it under a separate diagnostic that we could disable. I sent a patch that did so some time ago but I guess Masahiro never applied it...
Consider dropping the changes that fix non-kprintf warnings and including that patch as part of this series.
Quoted 20 lines
--- drivers/platform/surface/surface3_power.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/platform/surface/surface3_power.c +1
-1@@ -245,7 +245,7 @@ static int mshw0011_bix(struct mshw0011_data *cdata, struct bix *bix)dev_err (&client->dev,"Error reading serial no: %d\n" , ret);return ret; }else { -snprintf (bix->serial,ARRAY_SIZE (bix->serial),"%3pE%6pE" , buf +7 , buf); +snprintf (bix->serial,ARRAY_SIZE (bix->serial),"%3pE%4pE" , buf +7 , buf); }/* get cycle count */ Signature · 1 line
2.39.2
Re: [PATCH 5/9] surface3_power: avoid format string truncation warning
Quoted 10 lines · Nathan Chancellor
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns about printing a pair of escaped strings into a buffer that is too short:
drivers/platform/surface/surface3_power.c:248:3: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation-non-kprintf]248 | snprintf(bix->serial, ARRAY_SIZE(bix->serial), "%3pE%6pE", buf + 7, buf); | ^
Quoted 5 lines
Quoted 5 lines
Change the format string two print two less bytes so it always fits. The string is still truncated, so there is no change in behavior, but the compiler no longer warns about it.
Fixes 85f7582cd484 ("platform/surface: Move Surface 3 Power OpRegion driver to platform/surface")
Hmm... Is it really a move patch (which by title should not have changed the contents)?
(I haven't looked into it, though.)
Quoted 8 lines
Quoted 5 lines
Signed-off-by Arnd Bergmann <[email protected]>---
Not entirely sure about this one, as I've never used escaped strings, and don't know if gcc is correct to warn here, or if the kernel defines it differently from the standard.As far as I understand it, this is a false positive because clang does not understand the kernel's %p extensions.
Yes, %pE here is special. Btw, what has already been discussed a long time is to have a validation plugin in the kernel to check those %p extensions, but IIUC nobody committed to it.
That said, the patch is most likely incorrect.
Quoted 10 lines
GCC does not warn for
overflow or truncation when %p is involved but the clang developers chose to intentionally deviate from GCC in that aspect while sticking it under a separate diagnostic that we could disable. I sent a patch that did so some time ago but I guess Masahiro never applied it...Consider dropping the changes that fix non-kprintf warnings and including that patch as part of this series.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 5/9] surface3_power: avoid format string truncation warning
In reply to Arnd BergmannHello,
just a nitpick:
Quoted 1 line · Arnd Bergmann
Change the format string two print two less bytes so it always fits. The string
s/two/to/
Quoted 2 lines
is still truncated, so there is no change in behavior, but the compiler no longer warns about it.
Best regards
Uwe
Signature · 2 lines
Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
PATCH 6/9 Input: IMS: fix printf string overflow
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang warns about a string overflow in this driver
drivers/input/misc/ims-pcu.c:1802:2: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation] drivers/input/misc/ims-pcu.c:1814:2: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation]
Make the buffer a little longer to ensure it always fits.
--- drivers/input/misc/ims-pcu.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/input/misc/ims-pcu.c +2 -2
@@ -42,8 +42,8 @@ struct ims_pcu_backlight {#define IMS_PCU_PART_NUMBER_LEN 15#define IMS_PCU_SERIAL_NUMBER_LEN 8#define IMS_PCU_DOM_LEN 8 -#define IMS_PCU_FW_VERSION_LEN (9 + 1) -#define IMS_PCU_BL_VERSION_LEN (9 + 1) +#define IMS_PCU_FW_VERSION_LEN 16 +#define IMS_PCU_BL_VERSION_LEN 16#define IMS_PCU_BL_RESET_REASON_LEN (2 + 1)#define IMS_PCU_PCU_B_DEVICE_ID 5
Signature · 1 line
2.39.2
Re: [PATCH 6/9] Input: IMS: fix printf string overflow
Quoted 11 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns about a string overflow in this driver
drivers/input/misc/ims-pcu.c:1802:2: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation]drivers/input/misc/ims-pcu.c:1814:2: error: 'snprintf' will always be truncated; specified size is 10, but format string expands to at least 12 [-Werror,-Wformat-truncation]Make the buffer a little longer to ensure it always fits.
Fixes 628329d52474 ("Input: add IMS Passenger Control Unit driver")Signed-off-by Arnd Bergmann <[email protected]>
Applied, thank you.
Signature · 1 line
Dmitry
PATCH 7/9 scsi: mylex: fix sysfs buffer lengths
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
The myrb and myrs drivers use an odd way of implementing their sysfs files, calling snprintf() with a fixed length of 32 bytes to print into a page sized buffer. One of the strings is actually longer than 32 bytes, which clang can warn about:
drivers/scsi/myrb.c:1906:10: error: 'snprintf' will always be truncated; specified size is 32, but format string expands to at least 34 [-Werror,-Wformat-truncation] drivers/scsi/myrs.c:1089:10: error: 'snprintf' will always be truncated; specified size is 32, but format string expands to at least 34 [-Werror,-Wformat-truncation]
These could all be plain sprintf() without a length as the buffer is always long enough. On the other hand, sysfs files should not be overly long either, so just double the length to make sure the longest strings don't get truncated here.
--- drivers/scsi/myrb.c | 20 ++++++++++---------- drivers/scsi/myrs.c | 24 ++++++++++++------------ 2 files changed, 22 insertions(+), 22 deletions(-)
drivers/scsi/myrb.c +10 -10
@@ -1775,9 +1775,9 @@ static ssize_t raid_state_show(struct device *dev, name =myrb_devstate_name (ldev_info->state);if (name) - ret =snprintf (buf,32 ,"%s\n" , name); + ret =snprintf (buf,64 ,"%s\n" , name);else - ret =snprintf (buf,32 ,"Invalid (%02X)\n" , + ret =snprintf (buf,64 ,"Invalid (%02X)\n" , ldev_info->state); }else {struct myrb_pdev_state *pdev_info = sdev->hostdata;
@@ -1796,9 +1796,9 @@ static ssize_t raid_state_show(struct device *dev,else name =myrb_devstate_name (pdev_info->state);if (name) - ret =snprintf (buf,32 ,"%s\n" , name); + ret =snprintf (buf,64 ,"%s\n" , name);else - ret =snprintf (buf,32 ,"Invalid (%02X)\n" , + ret =snprintf (buf,64 ,"Invalid (%02X)\n" , pdev_info->state); }return ret;
@@ -1886,11 +1886,11 @@ static ssize_t raid_level_show(struct device *dev, name =myrb_raidlevel_name (ldev_info->raid_level);if (!name) -return snprintf (buf,32 ,"Invalid (%02X)\n" , +return snprintf (buf,64 ,"Invalid (%02X)\n" , ldev_info->state); -return snprintf (buf,32 ,"%s\n" , name); +return snprintf (buf,64 ,"%s\n" , name); } -return snprintf (buf,32 ,"Physical Drive\n" ); +return snprintf (buf,64 ,"Physical Drive\n" ); }static DEVICE_ATTR_RO (raid_level);
@@ -1903,15 +1903,15 @@ static ssize_t rebuild_show(struct device *dev,unsigned char status;if (sdev->channel <myrb_logical_channel (sdev->host)) -return snprintf (buf,32 ,"physical device - not rebuilding\n" ); +return snprintf (buf,64 ,"physical device - not rebuilding\n" ); status =myrb_get_rbld_progress (cb, &rbld_buf);if (rbld_buf.ldev_num != sdev->id || status !=MYRB_STATUS_SUCCESS ) -return snprintf (buf,32 ,"not rebuilding\n" ); +return snprintf (buf,64 ,"not rebuilding\n" ); -return snprintf (buf,32 ,"rebuilding block %u of %u\n" , +return snprintf (buf,64 ,"rebuilding block %u of %u\n" , rbld_buf.ldev_size - rbld_buf.blocks_left, rbld_buf.ldev_size); }
drivers/scsi/myrs.c +12 -12
@@ -947,9 +947,9 @@ static ssize_t raid_state_show(struct device *dev, name =myrs_devstate_name (ldev_info->dev_state);if (name) - ret =snprintf (buf,32 ,"%s\n" , name); + ret =snprintf (buf,64 ,"%s\n" , name);else - ret =snprintf (buf,32 ,"Invalid (%02X)\n" , + ret =snprintf (buf,64 ,"Invalid (%02X)\n" , ldev_info->dev_state); }else {struct myrs_pdev_info *pdev_info;
@@ -958,9 +958,9 @@ static ssize_t raid_state_show(struct device *dev, pdev_info = sdev->hostdata; name =myrs_devstate_name (pdev_info->dev_state);if (name) - ret =snprintf (buf,32 ,"%s\n" , name); + ret =snprintf (buf,64 ,"%s\n" , name);else - ret =snprintf (buf,32 ,"Invalid (%02X)\n" , + ret =snprintf (buf,64 ,"Invalid (%02X)\n" , pdev_info->dev_state); }return ret;
@@ -1066,13 +1066,13 @@ static ssize_t raid_level_show(struct device *dev, ldev_info = sdev->hostdata; name =myrs_raid_level_name (ldev_info->raid_level);if (!name) -return snprintf (buf,32 ,"Invalid (%02X)\n" , +return snprintf (buf,64 ,"Invalid (%02X)\n" , ldev_info->dev_state); }else name =myrs_raid_level_name (MYRS_RAID_PHYSICAL ); -return snprintf (buf,32 ,"%s\n" , name); +return snprintf (buf,64 ,"%s\n" , name); }static DEVICE_ATTR_RO (raid_level);
@@ -1086,7 +1086,7 @@ static ssize_t rebuild_show(struct device *dev,unsigned char status;if (sdev->channel < cs->ctlr_info->physchan_present) -return snprintf (buf,32 ,"physical device - not rebuilding\n" ); +return snprintf (buf,64 ,"physical device - not rebuilding\n" ); ldev_info = sdev->hostdata; ldev_num = ldev_info->ldev_num;
@@ -1098,11 +1098,11 @@ static ssize_t rebuild_show(struct device *dev,return -EIO ; }if (ldev_info->rbld_active) { -return snprintf (buf,32 ,"rebuilding block %zu of %zu\n" , +return snprintf (buf,64 ,"rebuilding block %zu of %zu\n" , (size_t )ldev_info->rbld_lba, (size_t )ldev_info->cfg_devsize); }else -return snprintf (buf,32 ,"not rebuilding\n" ); +return snprintf (buf,64 ,"not rebuilding\n" ); } static ssize_t rebuild_store(struct device *dev,
@@ -1190,7 +1190,7 @@ static ssize_t consistency_check_show(struct device *dev,unsigned short ldev_num;if (sdev->channel < cs->ctlr_info->physchan_present) -return snprintf (buf,32 ,"physical device - not checking\n" ); +return snprintf (buf,64 ,"physical device - not checking\n" ); ldev_info = sdev->hostdata;if (!ldev_info)
@@ -1198,11 +1198,11 @@ static ssize_t consistency_check_show(struct device *dev, ldev_num = ldev_info->ldev_num;myrs_get_ldev_info (cs, ldev_num, ldev_info);if (ldev_info->cc_active) -return snprintf (buf,32 ,"checking block %zu of %zu\n" , +return snprintf (buf,64 ,"checking block %zu of %zu\n" , (size_t )ldev_info->cc_lba, (size_t )ldev_info->cfg_devsize);else -return snprintf (buf,32 ,"not checking\n" ); +return snprintf (buf,64 ,"not checking\n" ); } static ssize_t consistency_check_store(struct device *dev,
Signature · 1 line
2.39.2
Re: [PATCH 7/9] scsi: mylex: fix sysfs buffer lengths
Quoted 15 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
The myrb and myrs drivers use an odd way of implementing their sysfs files, calling
snprintf()with a fixed length of 32 bytes to print into a page sized buffer. One of the strings is actually longer than 32 bytes, which clang can warn about:
drivers/scsi/myrb.c:1906:10: error: 'snprintf' will always be truncated; specified size is 32, but format string expands to at least 34 [-Werror,-Wformat-truncation]drivers/scsi/myrs.c:1089:10: error: 'snprintf' will always be truncated; specified size is 32, but format string expands to at least 34 [-Werror,-Wformat-truncation]These could all be plain
sprintf()without a length as the buffer is always long enough. On the other hand, sysfs files should not be overly long either, so just double the length to make sure the longest strings don't get truncated here.
Quoted 8 lines
Fixes 77266186397c ("scsi: myrs: Add Mylex RAID controller (SCSI interface)")Fixes 081ff398c56c ("scsi: myrb: Add Mylex RAID controller (block interface)")Signed-off-by Arnd Bergmann <[email protected]>--- drivers/scsi/myrb.c | 20 ++++++++++---------- drivers/scsi/myrs.c | 24 ++++++++++++------------ 2 files changed, 22 insertions(+), 22 deletions(-)
Yeah, counting is hard ...
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
PATCH 8/9 ALSA: aoa: avoid false-positive format truncation warning
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
clang warns about what it interprets as a truncated snprintf:
sound/aoa/soundbus/i2sbus/core.c:171:6: error: 'snprintf' will always be truncated; specified size is 6, but format string expands to at least 7 [-Werror,-Wformat-truncation-non-kprintf]
The actual problem here is that it does not understand the special %pOFn format string and assumes that it is a pointer followed by the string "OFn", which would indeed not fit.
Slightly increasing the size of the buffer to its natural alignment avoids the warning, as it is now long enough for the correct and the incorrect interprations.
--- sound/aoa/soundbus/i2sbus/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
sound/aoa/soundbus/i2sbus/core.c +1 -1
@@ -158,7 +158,7 @@ static int i2sbus_add_dev(struct macio_dev *macio,struct device_node *child, *sound =NULL ;struct resource *r;int i, layout =0 , rlen, ok = force; -char node_name[6 ]; +char node_name[8 ];static const char *rnames[] = {"i2sbus: %pOFn (control)" ,"i2sbus: %pOFn (tx)" ,"i2sbus: %pOFn (rx)" };
Signature · 1 line
2.39.2
Takashi Iwai
AppliedRe: [PATCH 8/9] ALSA: aoa: avoid false-positive format truncation warning
Quoted 11 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
clang warns about what it interprets as a truncated snprintf:
sound/aoa/soundbus/i2sbus/core.c:171:6: error: 'snprintf' will always be truncated; specified size is 6, but format string expands to at least 7 [-Werror,-Wformat-truncation-non-kprintf]The actual problem here is that it does not understand the special %pOFn format string and assumes that it is a pointer followed by the string "OFn", which would indeed not fit.
Quoted 6 lines
Slightly increasing the size of the buffer to its natural alignment avoids the warning, as it is now long enough for the correct and the incorrect interprations.
Fixes b917d58dcfaa ("ALSA: aoa: Convert to using %pOFn instead of device_node.name")Signed-off-by Arnd Bergmann <[email protected]>
Applied this one now to sound.git tree. Thanks.
Takashi
PATCH 9/9 kbuild: enable -Wformat-truncation on clang
In reply to Arnd BergmannFrom: Arnd Bergmann <[email protected]>
This warning option still produces output on gcc but is now clean when building with clang, so enable it conditionally on the compiler for now.
As far as I can tell, the remaining warnings with gcc are the result of analysing the code more deeply across inlining, while clang only does this within a function.
--- scripts/Makefile.extrawarn | 2 ++ 1 file changed, 2 insertions(+)
scripts/Makefile.extrawarn +2 -0
@@ -99,7 +99,9 @@ KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) KBUILD_CFLAGS += $(call cc-disable-warning, packed-not-aligned) KBUILD_CFLAGS += $(call cc-disable-warning, format-overflow) +ifdef CONFIG_CC_IS_GCC KBUILD_CFLAGS += $(call cc-disable-warning, format-truncation) +endif KBUILD_CFLAGS += $(call cc-disable-warning, stringop-truncation) KBUILD_CFLAGS += -Wno-override-init # alias for -Wno-initializer-overrides in clang
Signature · 1 line
2.39.2
Jakub Kicinski
maintainerQuoted 2 lines · On Tue, 26 Mar 2024 23:37:59 +0100 Arnd Bergmann wrote:
I hope that the patches can get picked up by platform maintainers directly, so the final patch can go in later on.
platform == subsystem? :)
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Hello:
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:
Quoted 8 lines · On Tue, 26 Mar 2024 23:37:59 +0100 you wrote:
From: Arnd Bergmann <[email protected]>
With randconfig build testing, I found only eight files that produce warnings with clang when -Wformat-truncation is enabled. This means we can just turn it on by default rather than only enabling it for "make W=1".
[...]
Here is the summary with links:
- [2/9] enetc: avoid truncating error message
https://git.kernel.org/netdev/net-next/c/9046d581ed58 - [3/9] qed: avoid truncating work queue length https://git.kernel.org/netdev/net-next/c/954fd908f177 - [4/9] mlx5: avoid truncating error message https://git.kernel.org/netdev/net-next/c/b324a960354b
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Martin K. Petersen
AppliedRe: (subset) [PATCH 0/9] enabled -Wformat-truncation for clang
In reply to Arnd BergmannQuoted 10 lines · Arnd Bergmann
With randconfig build testing, I found only eight files that produce warnings with clang when -Wformat-truncation is enabled. This means we can just turn it on by default rather than only enabling it for "make W=1".
Unfortunately, gcc produces a lot more warnings when the option is enabled, so it's not yet possible to turn it on both both compilers.
[...]
Applied to 6.9/scsi-fixes, thanks!
[7/9] scsi: mylex: fix sysfs buffer lengths
https://git.kernel.org/mkp/scsi/c/1197c5b2099f
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering