Hi, Thomas
On 2023-07-03 16:06:47+0800, Zhangjin Wu wrote:
Hi, Willy
[..]
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;Are you sure the ENOENT is really correct? I played with this before and got ENOENT because before the chroot test we have a testcase that does chdir("/").
Yes, there are some chdir tests before chroot, it does answer why relative path not work and return ENOENT: no such file in the relative path changed by chdir(), it differs from the one in PWD environment variable.
And therefore the relative name in argv[0] was not resolving correctly anymore against the changed working directory.
(You can also test this by executing *only* the chroot test and it should work)
Yeah, If chdir() back to current path, it does work:
CASE_TEST(chroot_exe); chdir(getenv("PWD")); EXPECT_SYSER(1, chroot(exe), -1, ENOTDIR); break;
-->
11 chdir_root = 0 [OK] 12 chdir_dot = 0 [OK] 13 chdir_blah = -1 ENOENT [OK] 14 chmod_self = -1 EPERM [OK] 15 chmod_exe = 0 [OK] 16 chown_self = -1 EPERM [OK] 17 chroot_root [SKIPPED] 18 chroot_blah = -1 ENOENT [OK] 19 chroot_exe pwd: /home/ubuntu/Develop/src/examples/musl exe: ./nolibc-test = -1 ENOTDIR [OK]
In general chroot() should work just fine with relative paths.
it does work with relative path, to make sure argv0 always work as expected, an extra 'chdir()' may really required, or let's clean up the previous chdir() test cases to call chdir(getenv("PWD")) after every test.
CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); break; CASE_TEST(chdir_dot); EXPECT_SYSZR(1, chdir(".")); break; CASE_TEST(chdir_blah); EXPECT_SYSER(1, chdir("/blah"), -1, ENOENT); break;
perhaps only call 'chdir(getenv("PWD"))' after chdir_root() is enough, because the chdir(".") doesn't really change the directory:
CASE_TEST(chdir_root); EXPECT_SYSZR(1, chdir("/")); chdir(getenv("PWD")); break; CASE_TEST(chdir_dot); EXPECT_SYSZR(1, chdir(".")); break; CASE_TEST(chdir_blah); EXPECT_SYSER(1, chdir("/blah"), -1, ENOENT); break;
Which one do you prefer?
This is really a lot of complexity and discussion only to avoid depending on procfs for the tests.
Yes, one step further to find more interesting info, thanks a lot ;-)
Mixing chdir and relative path really need to be more careful.
Best regards, Zhangjin
Thomas