Hi, Willy
This v4 mainly uses the argv0 suggested by you, at the same time, a new
run-libc-test target is added for glibc and musl, and the RB_ flags are
added for nolibc to allow compile nolibc-test.c without <linux/reboot.h>
for glibc, musl and nolibc (mainly for musl-gcc, without -I
/path/to/sysroot).
This patchset is based on the 20230705-nolibc-series2 branch of nolibc
repo [2], it must be applied after our v6 __sysret series [3] (argv0
exported there) and Thomas' chmod_net removal patchset [4] (the new
chmod_argv0 is added at the same line of chmod_net, will conflict).
This patchset assumes the chmod_net removal patchset will be applied at
first, if not, the chmod_argv0 added alphabetically will not be applied.
Since our new chmod_argv0 is exactly added to replace chmod_net, so,
Willy, is it ok for you to at least apply the chmod_net removal patch
[5] before this patchset?
selftests/nolibc: drop test chmod_net
This patchset is tested together with the v6 __sysret series [3]:
arch/board | result
------------|------------
arm/vexpress-a9 | 142 test(s) passed, 1 skipped, 0 failed.
arm/virt | 142 test(s) passed, 1 skipped, 0 failed.
aarch64/virt | 142 test(s) passed, 1 skipped, 0 failed.
ppc/g3beige | not supported
ppc/ppce500 | not supported
i386/pc | 142 test(s) passed, 1 skipped, 0 failed.
x86_64/pc | 142 test(s) passed, 1 skipped, 0 failed.
mipsel/malta | 142 test(s) passed, 1 skipped, 0 failed.
loongarch64/virt | 142 test(s) passed, 1 skipped, 0 failed.
riscv64/virt | 142 test(s) passed, 1 skipped, 0 failed.
riscv32/virt | 0 test(s) passed, 0 skipped, 0 failed.
s390x/s390-ccw-virtio | 142 test(s) passed, 1 skipped, 0 failed.
If use tinyconfig + basic console options (means disable all of the
other options, include procfs, shmem, tmpfs, net and memfd_create, to
save test time, only randomly choose 4 archs):
...
LOG: testing report for loongarch64/virt:
15 chmod_self [SKIPPED]
16 chown_self [SKIPPED]
40 link_cross [SKIPPED]
0 -fstackprotector not supported [SKIPPED]
139 test(s) passed, 4 skipped, 0 failed.
See all results in /labs/linux-lab/logging/nolibc/loongarch64-virt-nolibc-test.log
LOG: testing summary:
arch/board | result
------------|------------
arm/vexpress-a9 | 139 test(s) passed, 4 skipped, 0 failed.
x86_64/pc | 139 test(s) passed, 4 skipped, 0 failed.
mipsel/malta | 139 test(s) passed, 4 skipped, 0 failed.
loongarch64/virt | 139 test(s) passed, 4 skipped, 0 failed.
Changes from v3 --> v4:
* selftests/nolibc: stat_fault: silence NULL argument warning with glibc
selftests/nolibc: gettid: restore for glibc and musl
selftests/nolibc: add _LARGEFILE64_SOURCE for musl
selftests/nolibc: fix up int_fast16/32_t test cases for musl
selftests/nolibc: fix up kernel parameters support
selftests/nolibc: link_cross: use /proc/self/cmdline
tools/nolibc: add rmdir() support
selftests/nolibc: add a new rmdir() test case
selftests/nolibc: fix up failures when CONFIG_PROC_FS=n
selftests/nolibc: prepare /tmp for tmpfs or ramfs
selftests/nolibc: vfprintf: remove MEMFD_CREATE dependency
No change.
* selftests/nolibc: add run-libc-test target
New run and report for glibc or musl. for musl, we can simply issue:
$ make run-libc-test CC=/path/to/musl-install/bin/musl-gcc
* tools/nolibc: types.h: add RB_ flags for reboot()
selftests/nolibc: prefer <sys/reboot.h> to <linux/reboot.h>
Required by musl to compile nolibc-test.c without -I/path/to/sysroot
* selftests/nolibc: chdir_root: restore current path after test
restore current path to prevent breakage of using relative path
* selftests/nolibc: stat_timestamps: remove procfs dependency
selftests/nolibc: chroot_exe: remove procfs dependency
selftests/nolibc: add chmod_argv0 test
use argv0 instead of '/init' as before.
Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/lkml/[email protected]/
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/wtarreau/nolibc.git
[3]: https://lore.kernel.org/lkml/[email protected]/
[4]: https://lore.kernel.org/lkml/[email protected]/
[5]: https://lore.kernel.org/lkml/[email protected]/
Zhangjin Wu (18):
selftests/nolibc: add run-libc-test target
selftests/nolibc: stat_fault: silence NULL argument warning with glibc
selftests/nolibc: gettid: restore for glibc and musl
selftests/nolibc: add _LARGEFILE64_SOURCE for musl
selftests/nolibc: fix up int_fast16/32_t test cases for musl
tools/nolibc: types.h: add RB_ flags for reboot()
selftests/nolibc: prefer <sys/reboot.h> to <linux/reboot.h>
selftests/nolibc: fix up kernel parameters support
selftests/nolibc: link_cross: use /proc/self/cmdline
tools/nolibc: add rmdir() support
selftests/nolibc: add a new rmdir() test case
selftests/nolibc: fix up failures when CONFIG_PROC_FS=n
selftests/nolibc: prepare /tmp for tmpfs or ramfs
selftests/nolibc: vfprintf: remove MEMFD_CREATE dependency
selftests/nolibc: chdir_root: restore current path after test
selftests/nolibc: stat_timestamps: remove procfs dependency
selftests/nolibc: chroot_exe: remove procfs dependency
selftests/nolibc: add chmod_argv0 test
tools/include/nolibc/sys.h | 23 ++++-
tools/include/nolibc/types.h | 12 ++-
tools/testing/selftests/nolibc/Makefile | 4 +
tools/testing/selftests/nolibc/nolibc-test.c | 88 +++++++++++++++-----
4 files changed, 104 insertions(+), 23 deletions(-)
--
2.25.1
Use another invalid address (void *)1 instead of NULL to silence this
compile warning with glibc:
$ make libc-test
CC libc-test
nolibc-test.c: In function ‘run_syscall’:
nolibc-test.c:622:49: warning: null argument where non-null required (argument 1) [-Wnonnull]
622 | CASE_TEST(stat_fault); EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break;
| ^~~~
nolibc-test.c:304:79: note: in definition of macro ‘EXPECT_SYSER2’
304 | do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syserr2(expr, expret, experr1, experr2, llen); } while (0)
| ^~~~
nolibc-test.c:622:33: note: in expansion of macro ‘EXPECT_SYSER’
622 | CASE_TEST(stat_fault); EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break;
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 06c561c4b09d..b4b64125d418 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -742,7 +742,7 @@ int run_syscall(int min, int max)
CASE_TEST(select_stdout); EXPECT_SYSNE(1, ({ fd_set fds; FD_ZERO(&fds); FD_SET(1, &fds); select(2, NULL, &fds, NULL, NULL); }), -1); break;
CASE_TEST(select_fault); EXPECT_SYSER(1, select(1, (void *)1, NULL, NULL, 0), -1, EFAULT); break;
CASE_TEST(stat_blah); EXPECT_SYSER(1, stat("/proc/self/blah", &stat_buf), -1, ENOENT); break;
- CASE_TEST(stat_fault); EXPECT_SYSER(1, stat(NULL, &stat_buf), -1, EFAULT); break;
+ CASE_TEST(stat_fault); EXPECT_SYSER(1, stat((void *)1, &stat_buf), -1, EFAULT); break;
CASE_TEST(stat_timestamps); EXPECT_SYSZR(1, test_stat_timestamps()); break;
CASE_TEST(symlink_root); EXPECT_SYSER(1, symlink("/", "/"), -1, EEXIST); break;
CASE_TEST(unlink_root); EXPECT_SYSER(1, unlink("/"), -1, EISDIR); break;
--
2.25.1
The vfprintf test case require to open a temporary file to write, the
old memfd_create() method is perfect but has strong dependency on
MEMFD_CREATE and also TMPFS or HUGETLBFS (see fs/Kconfig):
config MEMFD_CREATE
def_bool TMPFS || HUGETLBFS
And from v6.2, MFD_NOEXEC_SEAL must be passed for the non-executable
memfd, otherwise, The kernel warning will be output to the test result
like this:
Running test 'vfprintf'
0 emptymemfd_create() without MFD_EXEC nor MFD_NOEXEC_SEAL, pid=1 'init'
"" = "" [OK]
To avoid such warning and also to remove the MEMFD_CREATE dependency,
let's open a file from tmpfs directly.
The /tmp directory is used to detect the existing of tmpfs, if not
there, skip instead of fail.
And further, for pid == 1, the initramfs is loaded as ramfs, which can
be used as tmpfs, so, it is able to further remove TMPFS dependency too.
Suggested-by: Thomas Weißschuh <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 6b863f7b677c..494195890e4b 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -875,10 +875,10 @@ static int expect_vfprintf(int llen, size_t c, const char *expected, const char
FILE *memfile;
va_list args;
- fd = memfd_create("vfprintf", 0);
+ fd = open("/tmp", O_TMPFILE | O_EXCL | O_RDWR, 0600);
if (fd == -1) {
- pad_spc(llen, 64, "[FAIL]\n");
- return 1;
+ pad_spc(llen, 64, "[SKIPPED]\n");
+ return 0;
}
memfile = fdopen(fd, "w+");
--
2.25.1
create a /tmp directory and mount tmpfs there, if tmpfs is not
mountable, use ramfs as tmpfs.
tmpfs will be used instead of procfs for some tests.
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 5497ee86cf40..6b863f7b677c 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1063,6 +1063,10 @@ int prepare(void)
}
}
+ /* try to mount /tmp if not mounted, if not mountable, use ramfs as tmpfs */
+ mkdir("/tmp", 0755);
+ mount("none", "/tmp", "tmpfs", 0, 0);
+
return 0;
}
--
2.25.1
Since argv0 also works for CONFIG_PROC_FS=n, let's use it instead of
'/proc/self/exe'.
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 2065b5b0d171..c74b2af2414e 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -710,7 +710,7 @@ int run_syscall(int min, int max)
CASE_TEST(chown_self); EXPECT_SYSER(proc, chown("/proc/self", 0, 0), -1, EPERM); break;
CASE_TEST(chroot_root); EXPECT_SYSZR(euid0, chroot("/")); break;
CASE_TEST(chroot_blah); EXPECT_SYSER(1, chroot("/proc/self/blah"), -1, ENOENT); break;
- CASE_TEST(chroot_exe); EXPECT_SYSER(proc, chroot("/proc/self/exe"), -1, ENOTDIR); break;
+ CASE_TEST(chroot_exe); EXPECT_SYSER(1, chroot(argv0), -1, ENOTDIR); break;
CASE_TEST(close_m1); EXPECT_SYSER(1, close(-1), -1, EBADF); break;
CASE_TEST(close_dup); EXPECT_SYSZR(1, close(dup(0))); break;
CASE_TEST(dup_0); tmp = dup(0); EXPECT_SYSNE(1, tmp, -1); close(tmp); break;
--
2.25.1
'/proc/self/' is a good path which doesn't have stale time info but it
is only available for CONFIG_PROC_FS=y.
When CONFIG_PROC_FS=n, use argv0 instead of '/proc/self', use '/' for the
worst case.
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 0ed207495ea6..2065b5b0d171 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -584,7 +584,7 @@ static int test_stat_timestamps(void)
if (sizeof(st.st_atim.tv_sec) != sizeof(st.st_atime))
return 1;
- if (stat("/proc/self/", &st))
+ if (stat("/proc/self/", &st) && stat(argv0, &st) && stat("/", &st))
return 1;
if (st.st_atim.tv_sec != st.st_atime || st.st_atim.tv_nsec > 1000000000)
--
2.25.1
argv0 is readable and chmodable, let's use it for chmod test, but a safe
umask should be used, the readable and executable modes should be
reserved.
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index c74b2af2414e..10247fbbdace 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -706,6 +706,7 @@ int run_syscall(int min, int max)
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;
+ CASE_TEST(chmod_argv0); EXPECT_SYSZR(1, chmod(argv0, 0555)); break;
CASE_TEST(chmod_self); EXPECT_SYSER(proc, chmod("/proc/self", 0555), -1, EPERM); break;
CASE_TEST(chown_self); EXPECT_SYSER(proc, chown("/proc/self", 0, 0), -1, EPERM); break;
CASE_TEST(chroot_root); EXPECT_SYSZR(euid0, chroot("/")); break;
--
2.25.1
Hi Zhangjin,
On Sat, Jul 08, 2023 at 02:21:20AM +0800, Zhangjin Wu wrote:
> This patchset assumes the chmod_net removal patchset will be applied at
> first, if not, the chmod_argv0 added alphabetically will not be applied.
> Since our new chmod_argv0 is exactly added to replace chmod_net, so,
> Willy, is it ok for you to at least apply the chmod_net removal patch
> [5] before this patchset?
Sure, and thanks for the reminder, I had missed it in the middle of
the flood.
(...)
> LOG: testing summary:
>
> arch/board | result
> ------------|------------
> arm/vexpress-a9 | 139 test(s) passed, 4 skipped, 0 failed.
> x86_64/pc | 139 test(s) passed, 4 skipped, 0 failed.
> mipsel/malta | 139 test(s) passed, 4 skipped, 0 failed.
> loongarch64/virt | 139 test(s) passed, 4 skipped, 0 failed.
Great, I like this, thank you! That was precisely the purpose of the
"skipped" initially, not to be too strictly bound to a specific config.
Now it's indeed even more flexible and that's better.
Series queued as well, thank you!
Willy
Hi Zhangjin,
On Sat, Jul 08, 2023 at 02:38:57AM +0800, Zhangjin Wu wrote:
> create a /tmp directory and mount tmpfs there, if tmpfs is not
> mountable, use ramfs as tmpfs.
>
> tmpfs will be used instead of procfs for some tests.
Just curious, in which cases do you need this ? We're building an
initramfs for the root that's already read-write, so without mounting
anything you already have write access. I'm taking it anyway for now,
but if you figure it's not needed we can later drop it (or just drop
the mount part and keep mkdir).
Willy
Zhangjin,
for your convenience, I've queued your pending series and Thomas's patches
in the following order here: 20230709-nolibc-ser2-tom-syscall-configv4
series2, thomas' chmod_net and ioperm patches, your new syscall series,
and your tiny config version 4 series.
Thomas and Zhangjin, do not hesitate to let me know if I missed anything.
The two series I left aside for now are still just under discussion
(output reformating and reorganisation of the arch include files).
We're getting closer to having something done now!
Thanks!
Willy
Hi, Willy
> Hi Zhangjin,
>
> On Sat, Jul 08, 2023 at 02:38:57AM +0800, Zhangjin Wu wrote:
> > create a /tmp directory and mount tmpfs there, if tmpfs is not
> > mountable, use ramfs as tmpfs.
> >
> > tmpfs will be used instead of procfs for some tests.
>
> Just curious, in which cases do you need this ? We're building an
> initramfs for the root that's already read-write, so without mounting
> anything you already have write access. I'm taking it anyway for now,
> but if you figure it's not needed we can later drop it (or just drop
> the mount part and keep mkdir).
>
This "/tmp" directory is originally created to check the 'TMPFS' existence for
the old vfprintf/memfd_create (from old version of the minimal config support),
it is used to skip the whole vfprintf tests if the TMPFS (or HUGETLBFS) is not
there. BTW, Thomas's patch [1] shows, MEMFD_CREATE is able to work with ramfs
too.
And it is later used by the old chmod_tmpdir/chmod_tmpfile and chroot_tmpfile
too (from old version of the minimal config support), so, it is important to
align with the normal Linux systems to let "/tmp" means TMPFS mount.
Now, we use "/tmp" directly in vfprintf, and we use argv0 for chmod_exe and
chroot_exe, so, the only user of "/tmp" is vfprintf currently. In this case, it
is a simple normal writable directory to allow create tmp files there, so,
agree very much to only reserve the mkdir part:
/* create /tmp if not there. Silently fail otherwise */
mkdir("/tmp", 0755);
Another consideration before is whether it is required to be consistent with
the normal Linux systems, let the "/tmp" directory mounted as tmpfs at most of
the time, but "/tmp" means ramfs for CONFIG_TMPFS=n currently even mount it
explicitly (ramfs is a fallback of tmpfs in such case), so, this assumption of
"/tmp" means tmpfs is not true currently.
What I'm worried about is people in the future assume "/tmp" as tmpfs at the
first glance and use the features only provided by TMPFS but not provided by
RAMFS (I'm not sure which one they will use). so, I even tried to create a
"/tmp/tmpfs" flag for TMPFS and "/tmp/ramfs" flag for RAMFS before, since there
is no user to explicitly prefer TMPFS to RAMFS currently, at last, I removed
these flags from the sent patchsets. Based on the same logic, The removal of
tmpfs mount is of course ok.
So, Willy, is it ok for you to remove that mount line with corresponding update
of the commit message (and the subject title), or require me to send a revision
for this patch?
Best regards,
Zhangjin
---
[1]: https://lore.kernel.org/lkml/20230703224803.GF4378@monkey/
> Willy
Hi Zhangjin,
On Mon, Jul 10, 2023 at 01:06:00PM +0800, Zhangjin Wu wrote:
> > On Sat, Jul 08, 2023 at 02:38:57AM +0800, Zhangjin Wu wrote:
> > > create a /tmp directory and mount tmpfs there, if tmpfs is not
> > > mountable, use ramfs as tmpfs.
> > >
> > > tmpfs will be used instead of procfs for some tests.
> >
> > Just curious, in which cases do you need this ? We're building an
> > initramfs for the root that's already read-write, so without mounting
> > anything you already have write access. I'm taking it anyway for now,
> > but if you figure it's not needed we can later drop it (or just drop
> > the mount part and keep mkdir).
> >
>
> This "/tmp" directory is originally created to check the 'TMPFS' existence for
> the old vfprintf/memfd_create (from old version of the minimal config support),
> it is used to skip the whole vfprintf tests if the TMPFS (or HUGETLBFS) is not
> there. BTW, Thomas's patch [1] shows, MEMFD_CREATE is able to work with ramfs
> too.
OK but here we're neither using it nor even checking its success.
> And it is later used by the old chmod_tmpdir/chmod_tmpfile and chroot_tmpfile
> too (from old version of the minimal config support), so, it is important to
> align with the normal Linux systems to let "/tmp" means TMPFS mount.
I think I didn't explain myself well. I'm not contesting a writable /tmp,
I was asking why *tmpfs*, because we have a root on ramfs by default, so
when you create /tmp, the sole fact that it succeeds implies that whatever
you'll put into it will already work without having to remount a tmpfs
inside.
> Now, we use "/tmp" directly in vfprintf, and we use argv0 for chmod_exe and
> chroot_exe, so, the only user of "/tmp" is vfprintf currently. In this case, it
> is a simple normal writable directory to allow create tmp files there, so,
> agree very much to only reserve the mkdir part:
>
> /* create /tmp if not there. Silently fail otherwise */
> mkdir("/tmp", 0755);
OK, then I'll do that.
> Another consideration before is whether it is required to be consistent with
> the normal Linux systems, let the "/tmp" directory mounted as tmpfs at most of
> the time,
That's not what I'm seeing on most of the systems I'm having access to,
where /tmp is on a plain file system (either / or link to /var/tmp but
always on disk, likely due to the huge size of the stuff stored there
that is rarely used and that should not eat memory).
> but "/tmp" means ramfs for CONFIG_TMPFS=n currently even mount it
> explicitly (ramfs is a fallback of tmpfs in such case), so, this assumption of
> "/tmp" means tmpfs is not true currently.
>
> What I'm worried about is people in the future assume "/tmp" as tmpfs at the
> first glance and use the features only provided by TMPFS but not provided by
> RAMFS (I'm not sure which one they will use). so, I even tried to create a
> "/tmp/tmpfs" flag for TMPFS and "/tmp/ramfs" flag for RAMFS before, since there
> is no user to explicitly prefer TMPFS to RAMFS currently, at last, I removed
> these flags from the sent patchsets. Based on the same logic, The removal of
> tmpfs mount is of course ok.
Indeed, and also, please keep in mind what the purpose of nolibc-test is:
make sure that the syscalls wrappers we write do work as expected, in part
by allowing us to compare against another libc to figure whether it's
the libc, the test or the kernel that causes any difference. The rest is
purely out of scope. Thus it's not this test's business to verify that a
tmpfs is indeed present after trying to mount it under /tmp, however it's
this test business to make sure that options passed to mount() do work as
expected, and that when a writable area is needed for a test, a working
one is assigned. Thus for the specific case you mention, we don't care.
And I'd go further, there can and should be reasonable prerequisites to
run this test.
> So, Willy, is it ok for you to remove that mount line with corresponding update
> of the commit message (and the subject title), or require me to send a revision
> for this patch?
No worries, I've modified it accordingly with the following commit message,
just let me know if you want to change anything:
commit 11fddb386bd663a554cc08c5950d9da2c87a7267 (HEAD)
Author: Zhangjin Wu <[email protected]>
Date: Sat Jul 8 02:38:57 2023 +0800
selftests/nolibc: prepare /tmp for tests that need to write
create a /tmp directory. If it succeeds, the directory is writable,
which is normally the case when booted from an initramfs anyway.
This will be used instead of procfs for some tests.
Reviewed-by: Thomas Weißschuh <[email protected]>
Signed-off-by: Zhangjin Wu <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
[wt: removed the unneeded mount() call]
Signed-off-by: Willy Tarreau <[email protected]>
Thanks!
Willy
Hi, Willy
> Hi Zhangjin,
>
> On Mon, Jul 10, 2023 at 01:06:00PM +0800, Zhangjin Wu wrote:
> > > On Sat, Jul 08, 2023 at 02:38:57AM +0800, Zhangjin Wu wrote:
[...]
> > Now, we use "/tmp" directly in vfprintf, and we use argv0 for chmod_exe and
> > chroot_exe, so, the only user of "/tmp" is vfprintf currently. In this case, it
> > is a simple normal writable directory to allow create tmp files there, so,
> > agree very much to only reserve the mkdir part:
> >
> > /* create /tmp if not there. Silently fail otherwise */
> > mkdir("/tmp", 0755);
>
> OK, then I'll do that.
>
Thanks.
> > Another consideration before is whether it is required to be consistent with
> > the normal Linux systems, let the "/tmp" directory mounted as tmpfs at most of
> > the time,
>
> That's not what I'm seeing on most of the systems I'm having access to,
> where /tmp is on a plain file system (either / or link to /var/tmp but
> always on disk, likely due to the huge size of the stuff stored there
> that is rarely used and that should not eat memory).
>
> > but "/tmp" means ramfs for CONFIG_TMPFS=n currently even mount it
> > explicitly (ramfs is a fallback of tmpfs in such case), so, this assumption of
> > "/tmp" means tmpfs is not true currently.
> >
> > What I'm worried about is people in the future assume "/tmp" as tmpfs at the
> > first glance and use the features only provided by TMPFS but not provided by
> > RAMFS (I'm not sure which one they will use). so, I even tried to create a
> > "/tmp/tmpfs" flag for TMPFS and "/tmp/ramfs" flag for RAMFS before, since there
> > is no user to explicitly prefer TMPFS to RAMFS currently, at last, I removed
> > these flags from the sent patchsets. Based on the same logic, The removal of
> > tmpfs mount is of course ok.
>
> Indeed, and also, please keep in mind what the purpose of nolibc-test is:
> make sure that the syscalls wrappers we write do work as expected, in part
> by allowing us to compare against another libc to figure whether it's
> the libc, the test or the kernel that causes any difference. The rest is
> purely out of scope. Thus it's not this test's business to verify that a
> tmpfs is indeed present after trying to mount it under /tmp, however it's
> this test business to make sure that options passed to mount() do work as
> expected, and that when a writable area is needed for a test, a working
> one is assigned. Thus for the specific case you mention, we don't care.
> And I'd go further, there can and should be reasonable prerequisites to
> run this test.
>
Ok.
> > So, Willy, is it ok for you to remove that mount line with corresponding update
> > of the commit message (and the subject title), or require me to send a revision
> > for this patch?
>
> No worries, I've modified it accordingly with the following commit message,
> just let me know if you want to change anything:
>
> commit 11fddb386bd663a554cc08c5950d9da2c87a7267 (HEAD)
> Author: Zhangjin Wu <[email protected]>
> Date: Sat Jul 8 02:38:57 2023 +0800
>
> selftests/nolibc: prepare /tmp for tests that need to write
>
> create a /tmp directory. If it succeeds, the directory is writable,
> which is normally the case when booted from an initramfs anyway.
>
> This will be used instead of procfs for some tests.
>
> Reviewed-by: Thomas Weißschuh <[email protected]>
> Signed-off-by: Zhangjin Wu <[email protected]>
> Link: https://lore.kernel.org/lkml/[email protected]/
> [wt: removed the unneeded mount() call]
> Signed-off-by: Willy Tarreau <[email protected]>
>
Perfectly, Thanks a lot.
Best regards,
Zhangjin
> Thanks!
> Willy