search

linux-test-project / ltp

Linux Test Project (mailing list: https://lists.linux.it/listinfo/ltp)
https://linux-test-project.readthedocs.io/
GNU General Public License v2.0
2.31k stars 1.01k forks source link

doio.h: MAX_FNAME_LENGTH should be more than 128 #234

Open bentsi opened 6 years ago

bentsi commented 6 years ago

ltp/testcases/kernel/fs/doio/doio.h MAX_FNAME_LENGTH should have value greater than 128 or maximum allowed by the OS

vapier commented 6 years ago

what exactly is the problem you're trying to solve ?

bentsi commented 6 years ago

The problem here is that when setting TMPDIR env variable to path that exceeds 128 characters iogen crashes with coredump.

bentsi commented 6 years ago

I run iogen in path that exceeds 128 characters, and it crashes with coredump. The program should not crash with coredump in such case. The solution is or to check path length and exit with error or increase MAX_FNAME_LENGTH to some bigger value

metan-ucw commented 6 years ago

Actually both should be done, I will push a commit that increases the MAX_FNAME_LENGHT, that one is easy enough.

Looking at the code the offending part is likely the function parse_cmdline() and the code that does getcwd() into array of size of MAX_FNAME_LENGTH then strcat() the filename at the end.

However the whole code looks like it will need some love, some parts are duplicated, some should be removed completely, such as the parts that were written to support non-POSIX conforming system such as CRAY or SGI...

metan-ucw commented 6 years ago

And it looks like the tests start to fail if I increase the MAX_FNAME_LENGTH and recompile, so the value is possibly hardcoded somewhere in the code :-(.

metan-ucw commented 6 years ago

Okay it looks like we need to fit the structures to be less than PIPE_BUF which is the largest atomic write for a pipe. The minimal value for PIPE_BUF is 512, which is probably the reason why the paths are so short. We can bump it to something as 400 bytes safely and even to 4000 for Linux since PIPE_BUF is 4k there.