public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
* [LTP][PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
@ 2026-01-13 19:49 Al Viro
  2026-01-14  8:35 ` [LTP] [PATCH] " Andrea Cervesato
  2026-01-14  9:40 ` Cyril Hrubis
  0 siblings, 2 replies; 6+ messages in thread
From: Al Viro @ 2026-01-13 19:49 UTC (permalink / raw)
  To: ltp; +Cc: linux-fsdevel

	There are different causes of ENAMETOOLONG.  It might come from
filesystem rejecting an excessively long pathname component, but there's
also "pathname is longer than PATH_MAX bytes, including terminating NUL"
and that doesn't get checked anywhere.

	Ran into that when a braino in kernel patch broke that logics
(ending up with cutoff too low) and that didn't get caught by LTP run.

	Patch below adds the checks to one of the tests that do deal
with the other source of ENAMETOOLONG; it almost certainly not the
right use of infrastructure, though.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/testcases/kernel/syscalls/chdir/chdir04.c b/testcases/kernel/syscalls/chdir/chdir04.c
index 6e53b7fef..e8dd5121d 100644
--- a/testcases/kernel/syscalls/chdir/chdir04.c
+++ b/testcases/kernel/syscalls/chdir/chdir04.c
@@ -11,6 +11,8 @@
 #include "tst_test.h"
 
 static char long_dir[] = "abcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
+static char long_path[PATH_MAX+1];
+static char shorter_path[PATH_MAX];
 static char noexist_dir[] = "noexistdir";
 
 static struct tcase {
@@ -20,16 +22,23 @@ static struct tcase {
 	{long_dir, ENAMETOOLONG},
 	{noexist_dir, ENOENT},
 	{0, EFAULT}, // bad_addr
+	{long_path, ENAMETOOLONG},
+	{shorter_path, 0},
 };
 
 static void verify_chdir(unsigned int i)
 {
-	TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
+	if (tcases[i].exp_errno)
+		TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
+	else
+		TST_EXP_PASS(chdir(tcases[i].dir), "chdir()");
 }
 
 static void setup(void)
 {
 	tcases[2].dir = tst_get_bad_addr(NULL);
+	memset(long_path, '/', PATH_MAX);
+	memset(shorter_path, '/', PATH_MAX - 1);
 }
 
 static struct tst_test test = {

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
  2026-01-13 19:49 [LTP][PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX Al Viro
@ 2026-01-14  8:35 ` Andrea Cervesato
  2026-01-14  8:37   ` Andrea Cervesato
  2026-01-14 14:30   ` Al Viro
  2026-01-14  9:40 ` Cyril Hrubis
  1 sibling, 2 replies; 6+ messages in thread
From: Andrea Cervesato @ 2026-01-14  8:35 UTC (permalink / raw)
  To: Al Viro, ltp; +Cc: linux-fsdevel

Hi!

On Tue Jan 13, 2026 at 8:49 PM CET, Al Viro wrote:
> 	There are different causes of ENAMETOOLONG.  It might come from
> filesystem rejecting an excessively long pathname component, but there's
> also "pathname is longer than PATH_MAX bytes, including terminating NUL"
> and that doesn't get checked anywhere.
>
> 	Ran into that when a braino in kernel patch broke that logics
> (ending up with cutoff too low) and that didn't get caught by LTP run.
>
> 	Patch below adds the checks to one of the tests that do deal
> with the other source of ENAMETOOLONG; it almost certainly not the
> right use of infrastructure, though.

The description is not well formatted, spaces at the beginning of the
phrases should be removed.

Also, we can make it slightly more clear, by saying that error can be
caused by a path name that is bigger than NAME_MAX, if relative, or
bigger than PATH_MAX, if absolute (when we use '/').

In this test we only verifies if relative paths are longer than
NAME_MAX (we give 273 bytes instead of 255 max), but we don't test if
path name is bigger than PATH_MAX.

We should correctly distinguish these two cases inside the test with
proper names as well. Check below..

>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/testcases/kernel/syscalls/chdir/chdir04.c b/testcases/kernel/syscalls/chdir/chdir04.c
> index 6e53b7fef..e8dd5121d 100644
> --- a/testcases/kernel/syscalls/chdir/chdir04.c
> +++ b/testcases/kernel/syscalls/chdir/chdir04.c
> @@ -11,6 +11,8 @@
>  #include "tst_test.h"
>  
>  static char long_dir[] = "abcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> +static char long_path[PATH_MAX+1];
> +static char shorter_path[PATH_MAX];
>  static char noexist_dir[] = "noexistdir";

When it comes to syscall testing, it's better to use guarded buffers.
This is easy to do: please check tst_test.bufs usage in here:

https://linux-test-project.readthedocs.io/en/latest/developers/api_c_tests.html#guarded-buffers-introduction

Many old tests are not using these buffers, but it's better to
introduce them when a test is refactored or fixed, like in this case.

You need to define:

static char *long_rel_path;
static char *long_abs_path;

...

static void setup(void) {
	..
	// initialize long_rel_path content
	// initialize long_abs_path content
}

static struct tst_test test = {
	..
	.bufs = (struct tst_buffer []) {
		{&long_rel_path, .size = NAME_MAX + 10},
		{&long_abs_path, .size = PATH_MAX + 10},
		{}
	}
};

>  
>  static struct tcase {
> @@ -20,16 +22,23 @@ static struct tcase {
>  	{long_dir, ENAMETOOLONG},
>  	{noexist_dir, ENOENT},
>  	{0, EFAULT}, // bad_addr
> +	{long_path, ENAMETOOLONG},
> +	{shorter_path, 0},
>  };
>  
>  static void verify_chdir(unsigned int i)
>  {
> -	TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
> +	if (tcases[i].exp_errno)
> +		TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
> +	else
> +		TST_EXP_PASS(chdir(tcases[i].dir), "chdir()");

In this test we only verify errors, so TST_EXP_PASS is not needed.

-- 
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
  2026-01-14  8:35 ` [LTP] [PATCH] " Andrea Cervesato
@ 2026-01-14  8:37   ` Andrea Cervesato
  2026-01-14 14:30   ` Al Viro
  1 sibling, 0 replies; 6+ messages in thread
From: Andrea Cervesato @ 2026-01-14  8:37 UTC (permalink / raw)
  To: Andrea Cervesato, Al Viro, ltp; +Cc: linux-fsdevel

I forgot to mention that guarded buffers should be introduced for
`noexist_dir` as well.

Kind regards,
-- 
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
  2026-01-13 19:49 [LTP][PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX Al Viro
  2026-01-14  8:35 ` [LTP] [PATCH] " Andrea Cervesato
@ 2026-01-14  9:40 ` Cyril Hrubis
  1 sibling, 0 replies; 6+ messages in thread
From: Cyril Hrubis @ 2026-01-14  9:40 UTC (permalink / raw)
  To: Al Viro; +Cc: ltp, linux-fsdevel

Hi!
> 	There are different causes of ENAMETOOLONG.  It might come from
> filesystem rejecting an excessively long pathname component, but there's
> also "pathname is longer than PATH_MAX bytes, including terminating NUL"
> and that doesn't get checked anywhere.

We do have a couple of tests that checks that names over PATH_MAX are
rejected, there is no reason to add these kind of tests, however I do
not think that we tests that check that names that are just under the
limit work fine, that needs to be added.

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/testcases/kernel/syscalls/chdir/chdir04.c b/testcases/kernel/syscalls/chdir/chdir04.c
> index 6e53b7fef..e8dd5121d 100644
> --- a/testcases/kernel/syscalls/chdir/chdir04.c
> +++ b/testcases/kernel/syscalls/chdir/chdir04.c
> @@ -11,6 +11,8 @@
>  #include "tst_test.h"
>  
>  static char long_dir[] = "abcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyzabcdefghijklmnopqrstmnopqrstuvwxyz";
> +static char long_path[PATH_MAX+1];
> +static char shorter_path[PATH_MAX];
>  static char noexist_dir[] = "noexistdir";
>  
>  static struct tcase {
> @@ -20,16 +22,23 @@ static struct tcase {
>  	{long_dir, ENAMETOOLONG},
>  	{noexist_dir, ENOENT},
>  	{0, EFAULT}, // bad_addr
> +	{long_path, ENAMETOOLONG},

This test already exists in the form of long_dir just three lines above.

> +	{shorter_path, 0},

What about we add a separate test (chdir02.c) for paths that shouldn't
be rejected. Something as:

char path[PATH_MAX];
int i;

...
	for (i = 1; i < PATH_MAX; i++) {
		memset(path, 0, sizeof(path));
		memset(path, '/', i);
		TST_EXP_PASS(chdir(path), "chdir() len=%i", i);
	}
...


That would make sure that all lenghts of paths that are valid are
accepted.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
  2026-01-14  8:35 ` [LTP] [PATCH] " Andrea Cervesato
  2026-01-14  8:37   ` Andrea Cervesato
@ 2026-01-14 14:30   ` Al Viro
  2026-01-14 14:38     ` Andrea Cervesato
  1 sibling, 1 reply; 6+ messages in thread
From: Al Viro @ 2026-01-14 14:30 UTC (permalink / raw)
  To: Andrea Cervesato; +Cc: ltp, linux-fsdevel

On Wed, Jan 14, 2026 at 09:35:48AM +0100, Andrea Cervesato wrote:
> Hi!
> 
> On Tue Jan 13, 2026 at 8:49 PM CET, Al Viro wrote:
> > 	There are different causes of ENAMETOOLONG.  It might come from
> > filesystem rejecting an excessively long pathname component, but there's
> > also "pathname is longer than PATH_MAX bytes, including terminating NUL"
> > and that doesn't get checked anywhere.
> >
> > 	Ran into that when a braino in kernel patch broke that logics
> > (ending up with cutoff too low) and that didn't get caught by LTP run.
> >
> > 	Patch below adds the checks to one of the tests that do deal
> > with the other source of ENAMETOOLONG; it almost certainly not the
> > right use of infrastructure, though.
> 
> The description is not well formatted, spaces at the beginning of the
> phrases should be removed.
> 
> Also, we can make it slightly more clear, by saying that error can be
> caused by a path name that is bigger than NAME_MAX, if relative, or
> bigger than PATH_MAX, if absolute (when we use '/').

Huh?  Absolute pathname is the one that _starts_ with '/'; e.g. "../include"
is relative, not absolute, despite having a '/' in it.

> In this test we only verifies if relative paths are longer than
> NAME_MAX (we give 273 bytes instead of 255 max), but we don't test if
> path name is bigger than PATH_MAX.
> 
> We should correctly distinguish these two cases inside the test with
> proper names as well. Check below..

> https://linux-test-project.readthedocs.io/en/latest/developers/api_c_tests.html#guarded-buffers-introduction
> 
> Many old tests are not using these buffers, but it's better to
> introduce them when a test is refactored or fixed, like in this case.
> 
> You need to define:
> 
> static char *long_rel_path;
> static char *long_abs_path;
> 
> ...
> 
> static void setup(void) {
> 	..
> 	// initialize long_rel_path content
> 	// initialize long_abs_path content
> }
> 
> static struct tst_test test = {
> 	..
> 	.bufs = (struct tst_buffer []) {
> 		{&long_rel_path, .size = NAME_MAX + 10},
> 		{&long_abs_path, .size = PATH_MAX + 10},
> 		{}
> 	}
> };

> > -	TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
> > +	if (tcases[i].exp_errno)
> > +		TST_EXP_FAIL(chdir(tcases[i].dir), tcases[i].exp_errno, "chdir()");
> > +	else
> > +		TST_EXP_PASS(chdir(tcases[i].dir), "chdir()");
> 
> In this test we only verify errors, so TST_EXP_PASS is not needed.

Er...  Intent was to verify two things: that anything longer than PATH_MAX triggers
ENAMETOOLONG, but anything up to PATH_MAX does not.  Having a pathname of exactly
4095 '/' (or interleaved . and / in the same amount, etc.) be rejected with ENAMETOOLONG
is just as much of a failure as not triggering ENAMETOOLONG on anything longer...

FWIW, I considered something like
	mkdir("subdirectory", 0700);
concatenating enough copies of "subdirectory/../" to get just under PATH_MAX and appending
"././././././././" to the end, so that truncation to PATH_MAX and to PATH_MAX-1 would
both be otherwise valid paths; decided that it's better to keep it simpler - a pile of
slashes is easier to produce and would resolve to a valid directory if not for the
total length restrictions.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [LTP] [PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX
  2026-01-14 14:30   ` Al Viro
@ 2026-01-14 14:38     ` Andrea Cervesato
  0 siblings, 0 replies; 6+ messages in thread
From: Andrea Cervesato @ 2026-01-14 14:38 UTC (permalink / raw)
  To: Al Viro; +Cc: ltp, linux-fsdevel

>
> Er...  Intent was to verify two things: that anything longer than PATH_MAX triggers
> ENAMETOOLONG, but anything up to PATH_MAX does not.  Having a pathname of exactly
> 4095 '/' (or interleaved . and / in the same amount, etc.) be rejected with ENAMETOOLONG
> is just as much of a failure as not triggering ENAMETOOLONG on anything longer...

In this case we need a new test verifying that PATH_MAX is actually
handled well, as Cyril suggested. But in this test we should only
verifying errors.

>
> FWIW, I considered something like
> 	mkdir("subdirectory", 0700);
> concatenating enough copies of "subdirectory/../" to get just under PATH_MAX and appending
> "././././././././" to the end, so that truncation to PATH_MAX and to PATH_MAX-1 would
> both be otherwise valid paths; decided that it's better to keep it simpler - a pile of
> slashes is easier to produce and would resolve to a valid directory if not for the
> total length restrictions.

It's up to you how you create the string that will trigger the error.
Also, you probably need to take a look at tst_test.needs_tmpdir.

-- 
Andrea Cervesato
SUSE QE Automation Engineer Linux
andrea.cervesato@suse.com


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-01-14 14:38 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-13 19:49 [LTP][PATCH] lack of ENAMETOOLONG testcases for pathnames longer than PATH_MAX Al Viro
2026-01-14  8:35 ` [LTP] [PATCH] " Andrea Cervesato
2026-01-14  8:37   ` Andrea Cervesato
2026-01-14 14:30   ` Al Viro
2026-01-14 14:38     ` Andrea Cervesato
2026-01-14  9:40 ` Cyril Hrubis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox