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

[Kevin Brodsky <kevin.brodsky@arm.com>]

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)
 {


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp