Re: [LTP] [PATCH 3/3] Provide a PATH_MAX-long buffer when expecting ENAMETOOLONG

Kevin Brodsky
2023-10-24T09:07:59Z

On 23/10/2023 17:06, Cyril Hrubis wrote:
> Hi!
>>>> A number of tests check that syscalls manipulating paths return
>>>> -ENAMETOOLONG when the specified path is longer than allowed. There
>>>> are actually two ways this error can be triggered:
>>>>
>>>> 1. If the given string is longer than PATH_MAX, i.e. 4096 as far as
>>>> the kernel is concerned, then the getname() helper will fail and
>>>> the kernel will return -ENAMETOOLONG right away.
>>>>
>>>> 2. If the string fits in PATH_MAX, but the filesystem rejects the
>>>> path name, for instance because one of its components is longer
>>>> than the support file name length (e.g. 255 for ext4).
>>> Ideally we should have at least one test that would hit the 1. as well...
>> This is what patch 3 is meant to achieve: instead of hitting 2. we now
>> systematically hit 1.
> Sigh, I meant 2. I guess that we would have to loop over filesystems
> (easily done with .all_filesystems = 1) and pass very long filename. Or
> do we have such test already?
>
> Looking at our tests, the rename10.c is actually one of two tests that
> sets .all_fileystems and checks for ENAMETOOLONG. Looking at the
> filesystem limits, all seems to have limits that are <= 255 characters,
> the only problem is a definition of character. For utf8 character 255
> characters are around 1021 (including nul terminator). So I suppose that
> if we pass another buffer that is PATH_MAX in length and has PATH_MAX-1
> characters we should consistenly hit 2. Or do I miss something?

This is a good point, I didn't think about it this way. Your idea seems
sensible. With this patch we always hit 1. as we specify a string that
is longer than PATH_MAX. We could instead hit 2. without out-of-bound
access by specifying a string that is at most PATH_MAX in length
(including the null terminator), and at least the filesystem character
limit. Maybe something like the diff below (just tested it, that works
fine).

Kevin

-----8<-----

--- a/testcases/kernel/syscalls/rename/rename10.c
+++ b/testcases/kernel/syscalls/rename/rename10.c
@@ -18,7 +18,7 @@
#define MNT_POINT "mntpoint"
#define TEMP_FILE "tmpfile"

-static char long_path[PATH_MAX + 1] = {[0 ... PATH_MAX] = 'a'};
+static char long_path[PATH_MAX] = {[0 ... PATH_MAX - 2] = 'a',
[PATH_MAX - 1] = '\0'};

static void setup(void)
{