From: "Thomas Weißschuh" <thomas@t-8ch.de>
To: Zhangjin Wu <falcon@tinylab.org>
Cc: w@1wt.eu, arnd@arndb.de, david.laight@aculab.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-riscv@lists.infradead.org
Subject: Re: [PATCH v5 14/14] selftests/nolibc: add mmap and munmap test cases
Date: Mon, 3 Jul 2023 10:20:41 +0200 [thread overview]
Message-ID: <f74a85a5-ea9a-4819-a95e-aa9525bf84f9@t-8ch.de> (raw)
In-Reply-To: <20230703080647.491363-1-falcon@tinylab.org>
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("/").
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)
In general chroot() should work just fine with relative paths.
This is really a lot of complexity and discussion only to avoid
depending on procfs for the tests.
Thomas
next prev parent reply other threads:[~2023-07-03 8:20 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-19 15:40 [PATCH v4 00/10] tools/nolibc: add a new syscall helper Zhangjin Wu
2023-06-19 15:41 ` [PATCH v4 01/10] tools/nolibc: sys.h: add a syscall return helper Zhangjin Wu
2023-06-19 15:42 ` [PATCH v4 02/10] tools/nolibc: unistd.h: apply __sysret() helper Zhangjin Wu
2023-06-19 15:44 ` [PATCH v4 03/10] tools/nolibc: sys.h: " Zhangjin Wu
2023-06-19 15:45 ` [PATCH v4 04/10] tools/nolibc: unistd.h: reorder the syscall macros Zhangjin Wu
2023-06-19 15:47 ` [PATCH v4 05/10] tools/nolibc: add missing my_syscall6() for mips Zhangjin Wu
2023-06-19 15:48 ` [PATCH v4 06/10] tools/nolibc: __sysret: support syscalls who return a pointer Zhangjin Wu
2023-06-19 15:51 ` [PATCH v4 07/10] tools/nolibc: clean up mmap() support Zhangjin Wu
2023-06-21 18:48 ` Thomas Weißschuh
2023-06-22 19:08 ` Zhangjin Wu
2023-06-19 15:52 ` [PATCH v4 08/10] selftests/nolibc: add EXPECT_PTREQ, EXPECT_PTRNE and EXPECT_PTRER Zhangjin Wu
2023-06-19 15:54 ` [PATCH v4 09/10] selftests/nolibc: add sbrk_0 to test current brk getting Zhangjin Wu
2023-06-19 15:55 ` [PATCH v4 10/10] selftests/nolibc: add mmap and munmap test cases Zhangjin Wu
2023-06-19 16:14 ` Zhangjin Wu
2023-06-21 18:58 ` Thomas Weißschuh
2023-06-22 19:32 ` Zhangjin Wu
2023-06-22 19:56 ` Thomas Weißschuh
2023-06-24 7:47 ` Zhangjin Wu
2023-06-28 13:07 ` [PATCH v5 00/14] tools/nolibc: add a new syscall helper Zhangjin Wu
2023-06-28 13:08 ` [PATCH v5 01/14] tools/nolibc: sys.h: add a syscall return helper Zhangjin Wu
2023-06-28 13:11 ` [PATCH v5 02/14] tools/nolibc: unistd.h: apply __sysret() helper Zhangjin Wu
2023-06-28 13:13 ` [PATCH v5 03/14] tools/nolibc: sys.h: " Zhangjin Wu
2023-06-28 13:14 ` [PATCH v5 04/14] tools/nolibc: unistd.h: reorder the syscall macros Zhangjin Wu
2023-06-28 13:17 ` [PATCH v5 05/14] tools/nolibc: string.h: clean up multiple whitespaces with tab Zhangjin Wu
2023-06-28 13:19 ` [PATCH v5 06/14] tools/nolibc: arch-*.h: clean up multiple whitespaces Zhangjin Wu
2023-07-02 18:44 ` Willy Tarreau
2023-07-03 14:02 ` Zhangjin Wu
2023-06-28 13:22 ` [PATCH v5 07/14] tools/nolibc: arch-loongarch.h: shrink with SYSCALL_CLOBBERLIST Zhangjin Wu
2023-07-02 18:50 ` Willy Tarreau
2023-07-03 11:28 ` Zhangjin Wu
2023-06-28 13:31 ` [PATCH v5 08/14] tools/nolibc: arch-mips.h: " Zhangjin Wu
2023-06-28 13:37 ` [PATCH v5 09/14] tools/nolibc: add missing my_syscall6() for mips Zhangjin Wu
2023-07-02 18:55 ` Willy Tarreau
2023-07-03 10:13 ` Zhangjin Wu
2023-06-28 13:39 ` [PATCH v5 10/14] tools/nolibc: __sysret: support syscalls who return a pointer Zhangjin Wu
2023-07-02 19:17 ` Willy Tarreau
2023-07-03 8:36 ` Zhangjin Wu
2023-07-03 10:03 ` Willy Tarreau
2023-07-03 11:15 ` Zhangjin Wu
2023-06-28 13:41 ` [PATCH v5 11/14] tools/nolibc: clean up mmap() support Zhangjin Wu
2023-07-02 19:23 ` Willy Tarreau
2023-07-03 6:51 ` Zhangjin Wu
2023-06-28 13:44 ` [PATCH v5 12/14] selftests/nolibc: add EXPECT_PTREQ, EXPECT_PTRNE and EXPECT_PTRER Zhangjin Wu
2023-06-28 13:46 ` [PATCH v5 13/14] selftests/nolibc: add sbrk_0 to test current brk getting Zhangjin Wu
2023-06-28 13:51 ` [PATCH v5 14/14] selftests/nolibc: add mmap and munmap test cases Zhangjin Wu
2023-07-02 19:33 ` Willy Tarreau
2023-07-03 6:03 ` Zhangjin Wu
2023-07-03 7:25 ` Willy Tarreau
2023-07-03 8:06 ` Zhangjin Wu
2023-07-03 8:20 ` Thomas Weißschuh [this message]
2023-07-03 9:15 ` Zhangjin Wu
2023-07-03 9:56 ` Willy Tarreau
2023-07-03 11:24 ` Zhangjin Wu
2023-07-02 19:34 ` [PATCH v5 00/14] tools/nolibc: add a new syscall helper Willy Tarreau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f74a85a5-ea9a-4819-a95e-aa9525bf84f9@t-8ch.de \
--to=thomas@t-8ch.de \
--cc=arnd@arndb.de \
--cc=david.laight@aculab.com \
--cc=falcon@tinylab.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=w@1wt.eu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).