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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6vjo6wo5c5tadvhnrvo2affcqu2cf6ecrx4ol5jxl6mvz7vu3z@6wgvyptlpq4p>
Date: Thu, 24 Apr 2025 13:39:36 +0200
From: Jan Kara <jack@...e.cz>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Theodore Ts'o <tytso@....edu>, 
	Andreas Dilger <adilger.kernel@...ger.ca>, Arnd Bergmann <arnd@...db.de>, Jan Kara <jack@...e.cz>, 
	Zhang Yi <yi.zhang@...wei.com>, Ojaswin Mujoo <ojaswin@...ux.ibm.com>, 
	"Darrick J. Wong" <djwong@...nel.org>, "Matthew Wilcox (Oracle)" <willy@...radead.org>, 
	"Ritesh Harjani (IBM)" <ritesh.list@...il.com>, Shida Zhang <zhangshida@...inos.cn>, 
	Baokun Li <libaokun1@...wei.com>, Jann Horn <jannh@...gle.com>, Brian Foster <bfoster@...hat.com>, 
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext4: avoid -Wformat-security warning

On Wed 23-04-25 18:43:49, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.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@...db.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@...e.com>
SUSE Labs, CR