[PATCH] ext4: avoid -Wformat-security warning

[PATCH] ext4: avoid -Wformat-security warning

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

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.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/ext4/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 94c7d2d828a6..3cfb1b670ea4 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -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;
 }
 
-- 
2.39.5

Re: [PATCH] ext4: avoid -Wformat-security warning

[Jan Kara]

On Wed 23-04-25 18:43:49, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> 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.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

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

> ---
>  fs/ext4/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 94c7d2d828a6..3cfb1b670ea4 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -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;
>  }
>  
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: avoid -Wformat-security warning

[Arnd Bergmann]

On Thu, Apr 24, 2025, at 13:39, Jan Kara wrote:
> On Wed 23-04-25 18:43:49, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> 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.
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> 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

Re: [PATCH] ext4: avoid -Wformat-security warning

[Jan Kara]

On Thu 24-04-25 14:31:02, Arnd Bergmann wrote:
> On Thu, Apr 24, 2025, at 13:39, Jan Kara wrote:
> > On Wed 23-04-25 18:43:49, Arnd Bergmann wrote:
> >> From: Arnd Bergmann <arnd@arndb.de>
> >> 
> >> 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.
> >> 
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >
> > 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.

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:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: avoid -Wformat-security warning

[Theodore Ts'o]

On Wed, 23 Apr 2025 18:43:49 +0200, Arnd Bergmann wrote:
> 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,
-- 
Theodore Ts'o <tytso@mit.edu>