[PATCH next] ext4: Fix diagnostic printf formats

[PATCH next] ext4: Fix diagnostic printf formats

[david.laight.linux]

From: David Laight <david.laight.linux@gmail.com>

The formats for non-terminated names should be "%.*s" not "%*.s".
The kernel currently treats "%*.s" as equivalent to "%*s" whereas
userspace requires it be equivalent to "%*.0s".
Neither is correct here.

Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
 fs/ext4/namei.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index c4b5e252af0e..7aaf5fbd4498 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -647,7 +647,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 					/* Directory is not encrypted */
 					(void) ext4fs_dirhash(dir, de->name,
 						de->name_len, &h);
-					printk("%*.s:(U)%x.%u ", len,
+					printk("%.*s:(U)%x.%u ", len,
 					       name, h.hash,
 					       (unsigned) ((char *) de
 							   - base));
@@ -683,7 +683,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 						(void) ext4fs_dirhash(dir,
 							de->name,
 							de->name_len, &h);
-					printk("%*.s:(E)%x.%u ", len, name,
+					printk("%.*s:(E)%x.%u ", len, name,
 					       h.hash, (unsigned) ((char *) de
 								   - base));
 					fscrypt_fname_free_buffer(
@@ -694,7 +694,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 				char *name = de->name;
 				(void) ext4fs_dirhash(dir, de->name,
 						      de->name_len, &h);
-				printk("%*.s:%x.%u ", len, name, h.hash,
+				printk("%.*s:%x.%u ", len, name, h.hash,
 				       (unsigned) ((char *) de - base));
 #endif
 			}
-- 
2.39.5

Re: [PATCH next] ext4: Fix diagnostic printf formats

[Andy Shevchenko]

On Thu, Mar 26, 2026 at 08:18:04PM +0000, david.laight.linux@gmail.com wrote:

> The formats for non-terminated names should be "%.*s" not "%*.s".
> The kernel currently treats "%*.s" as equivalent to "%*s" whereas
> userspace requires it be equivalent to "%*.0s".
> Neither is correct here.

This entire code seems was never tested properly and it's a dead code
until one defines manually DX_DEBUG. It also has tons of plain printk()
calls that may behave differently if the first character is not printable
but maps to the level of printk().

I'm not sure how your patch helps with all that, but apparently the
printed data has to be NUL-terminated, otherwise I have no idea how
it was ever working without crashes.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH next] ext4: Fix diagnostic printf formats

[David Laight]

On Fri, 27 Mar 2026 12:48:56 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Thu, Mar 26, 2026 at 08:18:04PM +0000, david.laight.linux@gmail.com wrote:
> 
> > The formats for non-terminated names should be "%.*s" not "%*.s".
> > The kernel currently treats "%*.s" as equivalent to "%*s" whereas
> > userspace requires it be equivalent to "%*.0s".
> > Neither is correct here.  
> 
> This entire code seems was never tested properly and it's a dead code
> until one defines manually DX_DEBUG. It also has tons of plain printk()
> calls that may behave differently if the first character is not printable
> but maps to the level of printk().
> 
> I'm not sure how your patch helps with all that, but apparently the
> printed data has to be NUL-terminated, otherwise I have no idea how
> it was ever working without crashes.
> 

I noticed that as well.
I suspect it way have worked for the person that wrote it because the
name strings all happened to be NUL terminated.
There is certainly likely to be a '\0' before you 'fall off' mapped
memory and crash - so maybe they just ignored the extra characters.

Clearly the other option is to delete it all.
But regardless the format string is wrong.

	David
 

Re: [PATCH next] ext4: Fix diagnostic printf formats

[Andy Shevchenko]

On Fri, Mar 27, 2026 at 12:54:12PM +0000, David Laight wrote:
> On Fri, 27 Mar 2026 12:48:56 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > On Thu, Mar 26, 2026 at 08:18:04PM +0000, david.laight.linux@gmail.com wrote:
> > 
> > > The formats for non-terminated names should be "%.*s" not "%*.s".
> > > The kernel currently treats "%*.s" as equivalent to "%*s" whereas
> > > userspace requires it be equivalent to "%*.0s".
> > > Neither is correct here.  
> > 
> > This entire code seems was never tested properly and it's a dead code
> > until one defines manually DX_DEBUG. It also has tons of plain printk()
> > calls that may behave differently if the first character is not printable
> > but maps to the level of printk().
> > 
> > I'm not sure how your patch helps with all that, but apparently the
> > printed data has to be NUL-terminated, otherwise I have no idea how
> > it was ever working without crashes.
> 
> I noticed that as well.
> I suspect it way have worked for the person that wrote it because the
> name strings all happened to be NUL terminated.
> There is certainly likely to be a '\0' before you 'fall off' mapped
> memory and crash - so maybe they just ignored the extra characters.
> 
> Clearly the other option is to delete it all.

I would go for the history of the change and if it's old enough and not
mentioned in any Documentation or not-so-old email thread, kill all that
for good. But better to hear the ext4 maintainers first.

> But regardless the format string is wrong.

P.S. A bit of off-topic, have you seen this?
https://elixir.bootlin.com/linux/v7.0-rc5/source/kernel/stacktrace.c#L33
Is it correct use of %c?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH next] ext4: Fix diagnostic printf formats

[David Laight]

On Fri, 27 Mar 2026 16:12:38 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

...
> P.S. A bit of off-topic, have you seen this?
> https://elixir.bootlin.com/linux/v7.0-rc5/source/kernel/stacktrace.c#L33
> Is it correct use of %c?
> 

Works with glibc (or, rather, with whichever libc the shell I'm using
is linked against):

$ printf '|%*c|\n' 5 x
|    x|
$

'man fprintf' tends to agree.
Left justify also works, either "%-*c" or passing -5.

The 'fun' starts if you print a zero with %c in the middle of some output.

I know some compilers have supported: int c = 'abcd';
But I can't remember whether the value could be printed with %4c.
I do remember that the value ended up byteswapped in memory on both
x86 and sparc.

	David

Re: [PATCH next] ext4: Fix diagnostic printf formats

[Theodore Tso]

On Fri, Mar 27, 2026 at 04:12:38PM +0200, Andy Shevchenko wrote:
> > > I'm not sure how your patch helps with all that, but apparently the
> > > printed data has to be NUL-terminated, otherwise I have no idea how
> > > it was ever working without crashes.
> > 
> > I noticed that as well.
> > I suspect it way have worked for the person that wrote it because the
> > name strings all happened to be NUL terminated.
> > There is certainly likely to be a '\0' before you 'fall off' mapped
> > memory and crash - so maybe they just ignored the extra characters.
> > 
> > Clearly the other option is to delete it all.
> 
> I would go for the history of the change and if it's old enough and not
> mentioned in any Documentation or not-so-old email thread, kill all that
> for good. But better to hear the ext4 maintainers first.

This is code that can only be manually enabled by adding a

#define DX_DEBUG

to the sources; it's not anything that users can configure using
Kconfig.  It *has* been used relatively recently, when developers
added support for three level htree directories.  I'm not sure why
they didn't run into the NULL termination issue, but since it is handy
to have the debugging code for developers' use, my preference would be
to keep the code and fix it up the problems.

   	    	     	    - Ted