Skip to contentSkip to conversation

ext4: avoid -Wformat-security warning

PATCH Applied Arnd Bergmann 4 participants 5 messages Apr 23, 2025 – May 20, 2025 ext4 file system

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ext4: avoid -Wformat-security warning

From: Arnd Bergmann <[email protected]>

check_igot_inode() prints a variable string, which causes a harmless warning with 'make W=1':

fs/ext4/inode.c:4763:45: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
4763 | ext4_error_inode(inode, function, line, 0, err_str);

Use a trivial "%s" format string instead.

---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ext4/inode.c +1 -1
index 94c7d2d828a6..3cfb1b670ea4 100644
@@ -4760,7 +4760,7 @@ static int check_igot_inode(struct inode *inode, ext4_iget_flags flags,
 	return 0;
 
 error:
-	ext4_error_inode(inode, function, line, 0, err_str);
+	ext4_error_inode(inode, function, line, 0, "%s", err_str);
 	return -EFSCORRUPTED;
 }
 
Signature · 1 line
2.39.5
reviewer
#1 lore raw
Quoted 11 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

check_igot_inode() prints a variable string, which causes a harmless warning with 'make W=1':

fs/ext4/inode.c:4763:45: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
4763 | ext4_error_inode(inode, function, line, 0, err_str);

Use a trivial "%s" format string instead.

Frankly I don't care much either way but if my memory serves me well year or two ago someone was going through the kernel and was replacing pointless ("%s", str) cases with printing the string directly. So we should make up our minds how we want this... In my opinion this is one of the warnings which may be useful but have false positives too often (like here where err_str is just a selection from several string literals) so I'm not sure it's worth the effort to try to silence it.

Honza

Quoted 20 lines
---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/ext4/inode.c +1 -1
index 94c7d2d828a6..3cfb1b670ea4 100644
@@ -4760,7 +4760,7 @@ static int check_igot_inode(struct inode *inode, ext4_iget_flags flags,
 	return 0;
 
 error:
-	ext4_error_inode(inode, function, line, 0, err_str);
+	ext4_error_inode(inode, function, line, 0, "%s", err_str);
 	return -EFSCORRUPTED;
 }
 
Signature · 1 line
2.39.5
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
#2 lore raw
Quoted 13 lines · Jan Kara
Quoted 11 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

check_igot_inode() prints a variable string, which causes a harmless warning with 'make W=1':

fs/ext4/inode.c:4763:45: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
4763 | ext4_error_inode(inode, function, line, 0, err_str);

Use a trivial "%s" format string instead.

Quoted 7 lines

Frankly I don't care much either way but if my memory serves me well year or two ago someone was going through the kernel and was replacing pointless ("%s", str) cases with printing the string directly. So we should make up our minds how we want this... In my opinion this is one of the warnings which may be useful but have false positives too often (like here where err_str is just a selection from several string literals) so I'm not sure it's worth the effort to try to silence it.

I had a look at the git log now and see no evidence of those patches getting merged, but plenty of patches going the same way as my patch.

I found one patch that removes a "%s" in the last decade in favor of directly using a string variable, but it's part of a larger rework and seems to have been inadvertent rather than as a directed change:

60a883d119ab ("spi: use kthread_create_worker() helper")

I have patches for all remaining -Wformat-security warnings, in 117 files. I have not tried to split that up and add individual changelog texts but instead just send patches for regressions at the moment.

Arnd

8<---

# full list of files with warnings

arch/x86/kernel/cpu/mce/core.c arch/x86/kernel/e820.c
arch/x86/xen/smp_pv.c drivers/accel/habanalabs/common/device.c drivers/accel/habanalabs/gaudi/gaudi.c drivers/block/aoe/aoechr.c
drivers/bus/ti-sysc.c
drivers/cdrom/cdrom.c
drivers/char/mem.c
drivers/devfreq/sun8i-a33-mbus.c drivers/dma-buf/dma-heap.c drivers/firmware/arm_scmi/notify.c drivers/firmware/arm_scmi/transports/virtio.c drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c drivers/gpu/drm/amd/amdkfd/kfd_process.c drivers/gpu/drm/arm/display/komeda/komeda_event.c drivers/gpu/drm/etnaviv/etnaviv_gpu.c drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c drivers/gpu/drm/rockchip/rockchip_drm_vop2.c drivers/gpu/drm/tests/drm_plane_helper_test.c drivers/gpu/drm/vc4/vc4_crtc.c drivers/gpu/drm/vmwgfx/vmwgfx_gmrid_manager.c drivers/i3c/master.c
drivers/iio/adc/aspeed_adc.c drivers/iio/industrialio-core.c drivers/iio/trigger/iio-trig-loop.c drivers/infiniband/core/device.c drivers/infiniband/hw/hfi1/device.c drivers/infiniband/hw/hfi1/msix.c drivers/infiniband/hw/mlx4/mcg.c drivers/infiniband/hw/qib/qib_file_ops.c drivers/infiniband/ulp/rtrs/rtrs-clt.c drivers/input/tablet/aiptek.c drivers/iommu/arm/arm-smmu/qcom_iommu.c drivers/iommu/exynos-iommu.c drivers/iommu/irq_remapping.c drivers/iommu/mtk_iommu_v1.c drivers/iommu/omap-iommu.c drivers/iommu/sprd-iommu.c drivers/iommu/sun50i-iommu.c drivers/iommu/tegra-smmu.c
drivers/md/dm-raid.c
drivers/media/dvb-core/dvbdev.c drivers/media/pci/cx23885/cx23885-dvb.c drivers/media/pci/netup_unidvb/netup_unidvb_core.c drivers/media/pci/saa7164/saa7164-dvb.c drivers/media/pci/smipcie/smipcie-main.c drivers/media/pci/zoran/zoran_card.c drivers/media/test-drivers/visl/visl-dec.c drivers/media/usb/dvb-usb-v2/rtl28xxu.c drivers/media/usb/dvb-usb/dib0700_devices.c drivers/media/usb/pvrusb2/pvrusb2-hdw.c drivers/media/usb/pvrusb2/pvrusb2-std.c drivers/media/v4l2-core/v4l2-spi.c drivers/mtd/mtdcore.c drivers/net/dsa/hirschmann/hellcreek_ptp.c drivers/net/dsa/sja1105/sja1105_spi.c drivers/net/ethernet/netronome/nfp/devlink_param.c drivers/net/ethernet/netronome/nfp/nfp_main.c drivers/net/ieee802154/adf7242.c drivers/net/wireless/intel/ipw2x00/libipw_wx.c drivers/nvme/host/sysfs.c
drivers/of/module.c drivers/perf/arm_cspmu/arm_cspmu.c drivers/platform/mellanox/mlxbf-pmc.c drivers/platform/mellanox/mlxreg-hotplug.c drivers/platform/mellanox/mlxreg-io.c drivers/platform/x86/intel/int3472/clk_and_regulator.c drivers/platform/x86/intel/int3472/discrete.c drivers/platform/x86/siemens/simatic-ipc.c drivers/platform/x86/x86-android-tablets/core.c drivers/pnp/interface.c drivers/power/supply/ds2760_battery.c drivers/pwm/core.c drivers/regulator/da9121-regulator.c drivers/remoteproc/remoteproc_core.c drivers/scsi/lpfc/lpfc_scsi.c drivers/scsi/lpfc/lpfc_sli.c drivers/scsi/qla2xxx/qla_init.c drivers/soc/qcom/qcom_aoss.c drivers/spi/spi-lantiq-ssc.c
drivers/spi/spi.c drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c drivers/target/iscsi/iscsi_target_configfs.c drivers/tty/serial/pch_uart.c
drivers/usb/phy/phy.c
drivers/usb/typec/tcpm/tcpm.c
fs/nfs/nfs42xattr.c
fs/ocfs2/dlm/dlmdomain.c
fs/quota/dquot.c
fs/xfs/xfs_buf_item_recover.c
include/linux/kernel.h
kernel/power/suspend_test.c
kernel/rcu/tree.c kernel/trace/preemptirq_delay_test.c kernel/trace/rv/reactor_panic.c kernel/trace/rv/reactor_printk.c lib/kunit/string-stream.c
lib/tests/scanf_kunit.c
mm/backing-dev.c
net/core/filter.c
net/rds/transport.c
net/tipc/netlink_compat.c
scripts/Makefile.extrawarn
sound/core/control.c
sound/core/control_led.c sound/core/seq/seq_clientmgr.c sound/core/seq/seq_ump_client.c
sound/core/sound.c
sound/drivers/opl3/opl3_seq.c sound/pci/hda/hda_bind.c sound/pci/korg1212/korg1212.c
sound/pci/rme32.c
sound/pci/rme96.c
sound/soc/fsl/imx-pcm-rpmsg.c sound/soc/sof/intel/hda-codec.c sound/usb/mixer_quirks.c

reviewer Reviewed-by
#3 lore raw
Quoted 22 lines · Arnd Bergmann
Quoted 13 lines · Jan Kara
Quoted 11 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

check_igot_inode() prints a variable string, which causes a harmless warning with 'make W=1':

fs/ext4/inode.c:4763:45: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
4763 | ext4_error_inode(inode, function, line, 0, err_str);

Use a trivial "%s" format string instead.

Quoted 7 lines

Frankly I don't care much either way but if my memory serves me well year or two ago someone was going through the kernel and was replacing pointless ("%s", str) cases with printing the string directly. So we should make up our minds how we want this... In my opinion this is one of the warnings which may be useful but have false positives too often (like here where err_str is just a selection from several string literals) so I'm not sure it's worth the effort to try to silence it.

Quoted 3 lines

I had a look at the git log now and see no evidence of those patches getting merged, but plenty of patches going the same way as my patch.

OK, so maybe I'm just confused or the patch indeed got discarded. Thanks for having a look! With this clarification feel free to add:

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
maintainer Applied
#4 lore raw
In reply to Arnd Bergmann
Quoted 9 lines · Arnd Bergmann

check_igot_inode() prints a variable string, which causes a harmless warning with 'make W=1':

fs/ext4/inode.c:4763:45: error: format string is not a string literal (potentially insecure) [-Werror,-Wformat-security]
4763 | ext4_error_inode(inode, function, line, 0, err_str);

Use a trivial "%s" format string instead.

[...]

Applied, thanks!

[1/1] ext4: avoid -Wformat-security warning

      commit: d612a07931e261c537978aad096e7340b687cd0c

Best regards,

Signature · 1 line
Theodore Ts'o <[email protected]>