Hi, Willy
On Mon, Jul 03, 2023 at 02:03:23PM +0800, Zhangjin Wu wrote:
- the others, for kernel without procfs let it pass even with 'worst case' kernel configs
You should include /dev/zero, which is commonly used to allocate anonymous memory and is more likely present and readable than any of the other files. And another file of choice is obviously argv[0] ;-) In this case you don't need any of the other extra ones. Thus I could suggest that you try in this order:
/dev/zero, /proc/self/exe, /proc/1/exe, argv[0]and be done with it. That doesn't prevent one from extending the list if really needed later, but I doubt it would be needed. Also, it's already arranged in a read-write, then read-only fallbacks mode, so if we later need to add more complex tests involving writes, the writable /dev/zero will have precedence.
Cool, both /dev/zero and argv[0] are very good candidates ;-)
Just verified both of them, works perfectly.
/dev/zero
we need to mknod it in prepare()
Indeed.
and also, in test_mmap_munmap(), stat() return a zero size of /dev/zero, in this case, we should assign a non-zero file_size ourselves.
- file_size = stat_buf.st_size; + /* file size of the special /dev/zero is 0, let's assign one manually */ + if (i == 0) + file_size = 3*page_size - 1; + else + file_size = stat_buf.st_size;OK but why this -1 ? That doesn't sound right, unless you explicitly want a file size that's not multiple of a page size for whatever reason ?
Just make sure the file size is a litle random, not just aligned with PAGE_SIZE, it is ok without '-1' ;-)
argv[0]
since nolibc has no realpath() currently, we can simply support the current path and the absolute path like this:
nolibc-test.c:
/* assigned as argv[0] in main(), will be used by some tests */ static char exe[PATH_MAX + 1];
main():
/* get absolute path of myself, nolibc has no realpath() currently */ #ifndef NOLIBC realpath(argv[0], exe); #else /* assume absolute path has no "./" */ if (strncmp(argv[0], "./", 2) != 0) strncat(exe, argv[0], strlen(argv[0]) + 1); else { pwd = getenv("PWD"); /* skip the ending '\0' */ strncat(exe, getenv("PWD"), strlen(pwd)); /* skip the first '.' */ strncat(exe, argv[0] + 1, strlen(argv[0])); } #endif
No, please, not like this. Just copy argv[0] (the pointer not the contents) and you're fine:
static const char *argv0; int main(int argc, char **argv, char **envp) { argv0 = argv[0]; ... }Nothing more, nothing less. Your program will always have its correct path when being called unless someone purposely forces it to something different, which is not our concern at all since this is a test program. And I'd rather call it "argv0" which exactly tells us what it contains than "exe" which can be misleading for that precise reason.
Yeah, locally, I just used a global argv0 pointer directly, but chroot_exe("./nolibc-test") not work when run 'libc-test' in host system, that is why I tried to get an absolute path ;-)
CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(exe), -1, ENOTDIR); break;
-->
19 chroot_exe = -1 ENOENT != (-1 ENOTDIR) [FAIL]
I removed the "proc ?" check manually to test if it also work with CONFIG_PROC_FS=n. it doesn't work, without absolute path, we need to add the ENOENT errno back to the errno check list.
I'm not sure if the other syscalls require an absolute path, so, the realpath() is called in this proposed method.
A full functional realpath() is a little complex, such as '../' support and even symlink support, let's delay its requirement at current stage ;-)
Please do not even engage into this, and keep in mind that the sole purpose of this test program is to help developers simply add tests to the set of existing ones. If the program becomes complex for doing stuff that is out of its scope, it will become much harder to extend and users will lose interest and motivation for updating it.
one or both of them may also help the other test cases:
chroot_exe (used '/init' before)
CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(proc ? "/proc/self/exe" : exe), -1, ENOTDIR); break;
chmod_exe (replace the one: chmod_tmpdir in another patchset)
CASE_TEST(chmod_exe); EXPECT_SYSZR(1, chmod(proc ? "/proc/self/exe" : exe, 0555)); break;
It should be safe enough to only remove the writable attribute for the test program.
stat_timestamps (used '/init' before)
if (stat("/proc/self/", &st) && stat(exe, &st) && stat("/dev/zero", &st) && stat("/", &st))
Indeed, why not!
Ok, without absolute path, the chroot_exe() will be changed back to something like this:
CASE_TEST(chroot_exe); EXPECT_SYSER2(1, chroot(proc ? "/proc/self/exe" : argv0), -1, ENOTDIR, ENOENT); break;
Best regards, Zhangjin
Will update the related patches with them.
OK thanks! Willy