Prefer /dev/shm over /tmp in utime family tests
Commit Message
From: Arjun Shankar <arjun@redhat.com>
tst-utime.c, tst-utimes.c, and tst-futimens.c create a temporary file in
/tmp to test Y2038 support in the corresponding functions. This causes
the tests to fail when /tmp is mounted as a filesystem that has a Y2038
bug; e.g. XFS.
This commit changes the tests to prefer using /dev/shm over /tmp.
/dev/shm is always mounted as tmpfs.
Since the change means a longer do_prepare function and duplicate code,
the do_prepare function is moved to a separate, new source file.
---
sysdeps/unix/sysv/linux/tst-futimens.c | 13 ++-----
sysdeps/unix/sysv/linux/tst-utime-prep.c | 44 ++++++++++++++++++++++++
sysdeps/unix/sysv/linux/tst-utime.c | 14 ++------
sysdeps/unix/sysv/linux/tst-utimes.c | 14 ++------
4 files changed, 53 insertions(+), 32 deletions(-)
create mode 100644 sysdeps/unix/sysv/linux/tst-utime-prep.c
Comments
On 08 Mar 2021 16:58, Arjun Shankar wrote:
> From: Arjun Shankar <arjun@redhat.com>
>
> tst-utime.c, tst-utimes.c, and tst-futimens.c create a temporary file in
> /tmp to test Y2038 support in the corresponding functions. This causes
> the tests to fail when /tmp is mounted as a filesystem that has a Y2038
> bug; e.g. XFS.
>
> This commit changes the tests to prefer using /dev/shm over /tmp.
> /dev/shm is always mounted as tmpfs.
>
> Since the change means a longer do_prepare function and duplicate code,
> the do_prepare function is moved to a separate, new source file.
this seems wrong to me to use /dev/shm as random scratch space. in CrOS
(and somewhat in systemd), we run services with isolated mount namespaces
where every service gets a unique /tmp, but /dev/shm has to be shared as
that's what processes use for shared mem APIs. if we devolve into using
/dev/shm as random scratch space, that weakens security boundaries. it
also ignores the user's configured tempdir settings.
assuming this is a transient issue that will resolve itself over time,
how about changing the logic to test the fs and then doing a SKIP if it
is known to be broken.
-mike
On 08/03/2021 14:07, Mike Frysinger via Libc-alpha wrote:
> On 08 Mar 2021 16:58, Arjun Shankar wrote:
>> From: Arjun Shankar <arjun@redhat.com>
>>
>> tst-utime.c, tst-utimes.c, and tst-futimens.c create a temporary file in
>> /tmp to test Y2038 support in the corresponding functions. This causes
>> the tests to fail when /tmp is mounted as a filesystem that has a Y2038
>> bug; e.g. XFS.
>>
>> This commit changes the tests to prefer using /dev/shm over /tmp.
>> /dev/shm is always mounted as tmpfs.
>>
>> Since the change means a longer do_prepare function and duplicate code,
>> the do_prepare function is moved to a separate, new source file.
>
> this seems wrong to me to use /dev/shm as random scratch space. in CrOS
> (and somewhat in systemd), we run services with isolated mount namespaces
> where every service gets a unique /tmp, but /dev/shm has to be shared as
> that's what processes use for shared mem APIs. if we devolve into using
> /dev/shm as random scratch space, that weakens security boundaries. it
> also ignores the user's configured tempdir settings.
>
> assuming this is a transient issue that will resolve itself over time,
> how about changing the logic to test the fs and then doing a SKIP if it
> is known to be broken.
> -mike
What instead we make libsupport to use builddir as the temporary base
directory instead? So all temporary files, directories, pipe, etc. would
be created and tests against a user-defined directory instead of a system
one.
* Adhemerval Zanella via Libc-alpha:
> What instead we make libsupport to use builddir as the temporary base
> directory instead? So all temporary files, directories, pipe, etc. would
> be created and tests against a user-defined directory instead of a system
> one.
It doesn't solve the issue with file system variance. It's worse than
/tmp because that's more likely to be on tmpfs. If we move the
directory into the build tree, everyone using XFS (that has the
forgetful VFS bug fixed) will end up with lots of FAILs.
Thanks,
Florian
On 08 Mar 2021 14:17, Adhemerval Zanella via Libc-alpha wrote:
> On 08/03/2021 14:07, Mike Frysinger via Libc-alpha wrote:
> > On 08 Mar 2021 16:58, Arjun Shankar wrote:
> >> From: Arjun Shankar <arjun@redhat.com>
> >>
> >> tst-utime.c, tst-utimes.c, and tst-futimens.c create a temporary file in
> >> /tmp to test Y2038 support in the corresponding functions. This causes
> >> the tests to fail when /tmp is mounted as a filesystem that has a Y2038
> >> bug; e.g. XFS.
> >>
> >> This commit changes the tests to prefer using /dev/shm over /tmp.
> >> /dev/shm is always mounted as tmpfs.
> >>
> >> Since the change means a longer do_prepare function and duplicate code,
> >> the do_prepare function is moved to a separate, new source file.
> >
> > this seems wrong to me to use /dev/shm as random scratch space. in CrOS
> > (and somewhat in systemd), we run services with isolated mount namespaces
> > where every service gets a unique /tmp, but /dev/shm has to be shared as
> > that's what processes use for shared mem APIs. if we devolve into using
> > /dev/shm as random scratch space, that weakens security boundaries. it
> > also ignores the user's configured tempdir settings.
> >
> > assuming this is a transient issue that will resolve itself over time,
> > how about changing the logic to test the fs and then doing a SKIP if it
> > is known to be broken.
>
> What instead we make libsupport to use builddir as the temporary base
> directory instead? So all temporary files, directories, pipe, etc. would
> be created and tests against a user-defined directory instead of a system
> one.
that would work, but i'm not sure it would necessarily address the issue
raised here (where the active FS has KI with y2038 dates).
-mike
On 08/03/2021 14:43, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> What instead we make libsupport to use builddir as the temporary base
>> directory instead? So all temporary files, directories, pipe, etc. would
>> be created and tests against a user-defined directory instead of a system
>> one.
>
> It doesn't solve the issue with file system variance. It's worse than
> /tmp because that's more likely to be on tmpfs. If we move the
> directory into the build tree, everyone using XFS (that has the
> forgetful VFS bug fixed) will end up with lots of FAILs.
But it gives to the user at least a way to avoid the issue by specifying where
to run and create all the files (even it is a loop device). I really think
that having a buggy FS is not the best way to check glibc functionality, where
part of tests will use a different system (where there is no direct requirement
such as POSIX semaphore due its implementation).
And for testsuite itself is slight better because it avoids possible spurious
errors with random name clashes (although unlikely) with system usage or
concurrent glibc tests usage.
* Adhemerval Zanella:
> On 08/03/2021 14:43, Florian Weimer wrote:
>> * Adhemerval Zanella via Libc-alpha:
>>
>>> What instead we make libsupport to use builddir as the temporary base
>>> directory instead? So all temporary files, directories, pipe, etc. would
>>> be created and tests against a user-defined directory instead of a system
>>> one.
>>
>> It doesn't solve the issue with file system variance. It's worse than
>> /tmp because that's more likely to be on tmpfs. If we move the
>> directory into the build tree, everyone using XFS (that has the
>> forgetful VFS bug fixed) will end up with lots of FAILs.
>
> But it gives to the user at least a way to avoid the issue by
> specifying where to run and create all the files (even it is a loop
> device). I really think that having a buggy FS is not the best way to
> check glibc functionality, where part of tests will use a different
> system (where there is no direct requirement such as POSIX semaphore
> due its implementation).
I want to avoid artificial barriers to new contributors, and additional
complications for my own work.
To provide some context: it is difficult for me to contribute to GCC
because the Dejagnu logs are hard to interpret and contain quite a few
expected failures which are not marked as such. We have some problems
with flaky tests, pretty much like everyone else, but the proposal here
is to add tests that fail consistently on common distributions. Quite
frankly, I find that approach rather hostile.
Thanks,
Florian
On 10/03/2021 05:55, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 08/03/2021 14:43, Florian Weimer wrote:
>>> * Adhemerval Zanella via Libc-alpha:
>>>
>>>> What instead we make libsupport to use builddir as the temporary base
>>>> directory instead? So all temporary files, directories, pipe, etc. would
>>>> be created and tests against a user-defined directory instead of a system
>>>> one.
>>>
>>> It doesn't solve the issue with file system variance. It's worse than
>>> /tmp because that's more likely to be on tmpfs. If we move the
>>> directory into the build tree, everyone using XFS (that has the
>>> forgetful VFS bug fixed) will end up with lots of FAILs.
>>
>> But it gives to the user at least a way to avoid the issue by
>> specifying where to run and create all the files (even it is a loop
>> device). I really think that having a buggy FS is not the best way to
>> check glibc functionality, where part of tests will use a different
>> system (where there is no direct requirement such as POSIX semaphore
>> due its implementation).
>
> I want to avoid artificial barriers to new contributors, and additional
> complications for my own work.
>
> To provide some context: it is difficult for me to contribute to GCC
> because the Dejagnu logs are hard to interpret and contain quite a few
> expected failures which are not marked as such. We have some problems
> with flaky tests, pretty much like everyone else, but the proposal here
> is to add tests that fail consistently on common distributions. Quite
> frankly, I find that approach rather hostile.
But it is rather different than using a clunky test framework where you
don't have a better option. It is a fundamental flaw in the kernel, where
users *should* be warned about and kernel developer should fix is asap,
not gloss over.
The idea of providing y2038 interfaces *now* is to actually stress the
possible scenarios where it would fail and warn users and developers of
potential issues.
I have tested it on different system, mostly based on Debian but also
one on SuSE. So this is more specific a RHEL issue that used XFS as
base and what I would expect is RHEL to fix asap instead of hiding it
on glibc testsuite.
Sorry, but I am unconvinced that we should handle it different than
what we do it now.
On 3/8/21 10:47 PM, Adhemerval Zanella via Libc-alpha wrote:
> What instead we make libsupport to use builddir as the temporary base
> directory instead? So all temporary files, directories, pipe, etc. would
> be created and tests against a user-defined directory instead of a system
> one.
>
Replacing tmpfs with literally anything else seems like a step forward
for another set of tests: the setuid tests for getenv and tunables. The
sgid functionality testing gets skipped because one cannot create a sgid
binary in tmpfs.
Siddhesh
@@ -22,9 +22,6 @@
#include <sys/stat.h>
#include <support/check.h>
#include <support/xunistd.h>
-#include <support/temp_file.h>
-
-static int temp_fd = -1;
/* struct timespec array with Y2038 threshold minus 2 and 1 seconds. */
const struct timespec t1[2] = { { 0x7FFFFFFE, 0 }, { 0x7FFFFFFF, 0 } };
@@ -35,13 +32,9 @@ const struct timespec t2[2] = { { 0x80000001ULL, 0 }, { 0x80000002ULL, 0 } };
/* struct timespec array around Y2038 threshold. */
const struct timespec t3[2] = { { 0x7FFFFFFE, 0 }, { 0x80000002ULL, 0 } };
-#define PREPARE do_prepare
-static void
-do_prepare (int argc, char *argv[])
-{
- temp_fd = create_temp_file ("futimensat", NULL);
- TEST_VERIFY_EXIT (temp_fd > 0);
-}
+/* The do_prepare function creates a temporary file to work with,
+ defining `int temp_fd' and `char *testfile'. */
+#include "tst-utime-prep.c"
static int
test_futimens_helper (const struct timespec *ts)
new file mode 100644
@@ -0,0 +1,44 @@
+/* do_prepare function for tests of the utime family of functions.
+ Copyright (C) 2021 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <sys/types.h>
+#include <support/check.h>
+#include <support/temp_file.h>
+#include <dirent.h>
+
+static int temp_fd = -1;
+char *testfile;
+
+#define PREPARE do_prepare
+static void
+do_prepare (int argc, char *argv[])
+{
+ /* Prefer using /dev/shm when it exists. It uses tmpfs, which is
+ Y2038-safe. /tmp could use, e.g. XFS, which presently has a Y2038 bug
+ triggering a test failure. */
+ DIR *d = opendir ("/dev/shm");
+ if (d != NULL)
+ {
+ temp_fd = create_temp_file_in_dir ("utime", "/dev/shm", &testfile);
+ closedir (d);
+ }
+ else
+ temp_fd = create_temp_file ("utime", &testfile);
+
+ TEST_VERIFY_EXIT (temp_fd > 0);
+}
@@ -24,10 +24,6 @@
#include <utime.h>
#include <support/check.h>
#include <support/xunistd.h>
-#include <support/temp_file.h>
-
-static int temp_fd = -1;
-char *testfile;
/* struct utimbuf with Y2038 threshold minus 2 and 1 seconds. */
const static struct utimbuf t1 = { 0x7FFFFFFE, 0x7FFFFFFF };
@@ -38,13 +34,9 @@ const static struct utimbuf t2 = { 0x80000001ULL, 0x80000002ULL };
/* struct utimbuf around Y2038 threshold. */
const static struct utimbuf t3 = { 0x7FFFFFFE, 0x80000002ULL };
-#define PREPARE do_prepare
-static void
-do_prepare (int argc, char *argv[])
-{
- temp_fd = create_temp_file ("utime", &testfile);
- TEST_VERIFY_EXIT (temp_fd > 0);
-}
+/* The do_prepare function creates a temporary file to work with,
+ defining `int temp_fd' and `char *testfile'. */
+#include "tst-utime-prep.c"
static int
test_utime_helper (const struct utimbuf *ut)
@@ -23,10 +23,6 @@
#include <sys/time.h>
#include <support/check.h>
#include <support/xunistd.h>
-#include <support/temp_file.h>
-
-static int temp_fd = -1;
-char *testfile;
/* struct timeval array with Y2038 threshold minus 2 and 1 seconds. */
const static struct timeval t1[2] = { { 0x7FFFFFFE, 0 }, { 0x7FFFFFFF, 0 } };
@@ -39,13 +35,9 @@ struct timeval t2[2] = { { 0x80000001ULL, 0 }, { 0x80000002ULL, 0 } };
const static
struct timeval t3[2] = { { 0x7FFFFFFE, 0 }, { 0x80000002ULL, 0 } };
-#define PREPARE do_prepare
-static void
-do_prepare (int argc, char *argv[])
-{
- temp_fd = create_temp_file ("utimes", &testfile);
- TEST_VERIFY_EXIT (temp_fd > 0);
-}
+/* The do_prepare function creates a temporary file to work with,
+ defining `int temp_fd' and `char *testfile'. */
+#include "tst-utime-prep.c"
static int
test_utime_helper (const struct timeval *tv)