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(-)
--
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]
From: 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(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index c278f8893042..8159b4c315b5 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -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;
--
2.39.2
From: 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.
Fixes: 70f0302b3f20 ("net/mlx5: Bridge, implement mdb offload")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
index 1b9bc32efd6f..c5ea1d1d2b03 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
@@ -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;
}
--
2.39.2
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.
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.
---
drivers/platform/surface/surface3_power.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c
index 4c0f92562a79..72f904761fde 100644
--- a/drivers/platform/surface/surface3_power.c
+++ b/drivers/platform/surface/surface3_power.c
@@ -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 */
--
2.39.2
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]>
---
drivers/input/misc/ims-pcu.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 6e8cc28debd9..80d16c92a08b 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -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
--
2.39.2
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.
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]>
---
sound/aoa/soundbus/i2sbus/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/aoa/soundbus/i2sbus/core.c b/sound/aoa/soundbus/i2sbus/core.c
index b8ff5cccd0c8..5431d2c49421 100644
--- a/sound/aoa/soundbus/i2sbus/core.c
+++ b/sound/aoa/soundbus/i2sbus/core.c
@@ -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)" };
--
2.39.2
From: 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.
Signed-off-by: Arnd Bergmann <[email protected]>
---
scripts/Makefile.extrawarn | 2 ++
1 file changed, 2 insertions(+)
diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index bff6c686df7c..aa1c716c4812 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -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
--
2.39.2
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.
Fixes: c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
index eb2297b37504..d35d2cf99998 100644
--- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
+++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
@@ -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;
--
2.39.2
From: 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.
Fixes: f968c56417f0 ("net: enetc: shorten enetc_setup_xdp_prog() error message to fit NETLINK_MAX_FMTMSG_LEN")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/net/ethernet/freescale/enetc/enetc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 9f07f4947b63..5c45f42232d3 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -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);
--
2.39.2
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.
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(-)
diff --git a/drivers/scsi/myrb.c b/drivers/scsi/myrb.c
index ca2e932dd9b7..f684eb5e0489 100644
--- a/drivers/scsi/myrb.c
+++ b/drivers/scsi/myrb.c
@@ -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);
}
diff --git a/drivers/scsi/myrs.c b/drivers/scsi/myrs.c
index a1eec65a9713..e824be9d9bbb 100644
--- a/drivers/scsi/myrs.c
+++ b/drivers/scsi/myrs.c
@@ -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,
--
2.39.2
Hi Arnd,
Thank you for the patch.
On Tue, Mar 26, 2024 at 11:38:00PM +0100, Arnd Bergmann wrote:
> 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.
>
> Fixes: c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index eb2297b37504..d35d2cf99998 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -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;
--
Regards,
Laurent Pinchart
On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> 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.
>
> 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...
https://lore.kernel.org/20231002-disable-wformat-truncation-overflow-non-kprintf-v1-1-35179205c8d9@kernel.org/
Consider dropping the changes that fix non-kprintf warnings and
including that patch as part of this series.
> ---
> drivers/platform/surface/surface3_power.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/surface/surface3_power.c b/drivers/platform/surface/surface3_power.c
> index 4c0f92562a79..72f904761fde 100644
> --- a/drivers/platform/surface/surface3_power.c
> +++ b/drivers/platform/surface/surface3_power.c
> @@ -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 */
> --
> 2.39.2
>
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? :)
On 3/26/24 23:38, Arnd Bergmann wrote:
> 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.
>
> 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 ...
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
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
On 3/26/24 23:38, Arnd Bergmann wrote:
> 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.
>
> Fixes: c5deac3c9b22 ("fbdev: sh_mobile_lcdc: Implement overlays support")
> Signed-off-by: Arnd Bergmann <[email protected]>
applied to fbdev git tree.
Thanks!
Helge
> ---
> drivers/video/fbdev/sh_mobile_lcdcfb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> index eb2297b37504..d35d2cf99998 100644
> --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c
> +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c
> @@ -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;
On Tue, 26 Mar 2024 23:38:07 +0100,
Arnd Bergmann wrote:
>
> 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.
>
> 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
On Wed, Mar 27, 2024 at 1:05 AM Nathan Chancellor <[email protected]> wrote:
> On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> > 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.
> > 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.)
> > 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.
> 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...
>
> https://lore.kernel.org/20231002-disable-wformat-truncation-overflow-non-kprintf-v1-1-35179205c8d9@kernel.org/
>
> Consider dropping the changes that fix non-kprintf warnings and
> including that patch as part of this series.
--
With Best Regards,
Andy Shevchenko
Hi,
>-----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 length
>
>From: 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(-)
>
>diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c
>b/drivers/net/ethernet/qlogic/qed/qed_main.c
>index c278f8893042..8159b4c315b5 100644
>--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
>+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
>@@ -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);
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
>
>- hwfn->slowpath_wq = alloc_workqueue(name, 0, 0);
> if (!hwfn->slowpath_wq) {
> DP_NOTICE(hwfn, "Cannot create slowpath
>workqueue\n");
> return -ENOMEM;
>--
>2.39.2
>
Hi,
>-----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 message
>
>From: 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.
>
>Fixes: 70f0302b3f20 ("net/mlx5: Bridge, implement mdb offload")
>Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Subbaraya Sundeep <[email protected]>
Thanks,
Sundeep
>---
> drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
>b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
>index 1b9bc32efd6f..c5ea1d1d2b03 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/bridge.c
>@@ -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;
> }
>--
>2.39.2
>
On Wed, Mar 27, 2024, at 15:04, Subbaraya Sundeep Bhatta wrote:
>>- 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?
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
Hi,
>-----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 length
>
>On Wed, Mar 27, 2024, at 15:04, Subbaraya Sundeep Bhatta wrote:
>
>>>- 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?
>
>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.
>
My bad. Got it
Thanks,
Sundeep
> Arnd
On Tue, Mar 26, 2024 at 11:38:05PM +0100, Arnd Bergmann wrote:
> 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.
--
Dmitry
Hello:
This series was applied to netdev/net-next.git (main)
by Jakub Kicinski <[email protected]>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Hello,
just a nitpick:
On Tue, Mar 26, 2024 at 11:38:04PM +0100, Arnd Bergmann wrote:
> Change the format string two print two less bytes so it always fits. The string
s/two/to/
> is still truncated, so there is no change in behavior, but the compiler no
> longer warns about it.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Tue, 26 Mar 2024 23:37:59 +0100, Arnd Bergmann wrote:
> 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
--
Martin K. Petersen Oracle Linux Engineering