'available_events' is actually not required by
'test.d/event/toplevel-enable.tc' and its Existence has been tested in
'test.d/00basic/basic4.tc'.
So the require of 'available_events' can be dropped and then we can add
'instance' flag to test 'test.d/event/toplevel-enable.tc' for instance.
Test result show as below:
# ./ftracetest test.d/event/toplevel-enable.tc
=== Ftrace unit tests ===
[1] event tracing - enable/disable with top level files [PASS]
[2] (instance) event tracing - enable/disable with top level files [PASS]
# of passed: 2
# of failed: 0
# of unresolved: 0
# of untested: 0
# of unsupported: 0
# of xfailed: 0
# of undefined(test bug): 0
Signed-off-by: Zheng Yejian <zhengyejian1(a)huawei.com>
---
tools/testing/selftests/ftrace/test.d/event/toplevel-enable.tc | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ftrace/test.d/event/toplevel-enable.tc b/tools/testing/selftests/ftrace/test.d/event/toplevel-enable.tc
index 93c10ea42a68..8b8e1aea985b 100644
--- a/tools/testing/selftests/ftrace/test.d/event/toplevel-enable.tc
+++ b/tools/testing/selftests/ftrace/test.d/event/toplevel-enable.tc
@@ -1,7 +1,8 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0
# description: event tracing - enable/disable with top level files
-# requires: available_events set_event events/enable
+# requires: set_event events/enable
+# flags: instance
do_reset() {
echo > set_event
--
2.25.1
Function instance_set() expects to enable event 'sched_switch', so we
should set 1 to its 'enable' file.
Testcase passed after this patch:
# ./ftracetest test.d/instances/instance-event.tc
=== Ftrace unit tests ===
[1] Test creation and deletion of trace instances while setting an event
[PASS]
# of passed: 1
# of failed: 0
# of unresolved: 0
# of untested: 0
# of unsupported: 0
# of xfailed: 0
# of undefined(test bug): 0
Signed-off-by: Zheng Yejian <zhengyejian1(a)huawei.com>
---
.../testing/selftests/ftrace/test.d/instances/instance-event.tc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
index 0eb47fbb3f44..42422e425107 100644
--- a/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
+++ b/tools/testing/selftests/ftrace/test.d/instances/instance-event.tc
@@ -39,7 +39,7 @@ instance_read() {
instance_set() {
while :; do
- echo 1 > foo/events/sched/sched_switch
+ echo 1 > foo/events/sched/sched_switch/enable
done 2> /dev/null
}
--
2.25.1
The most critical issue with vm.memfd_noexec=2 (the fact that passing
MFD_EXEC would bypass it entirely[1]) has been fixed in Andrew's
tree[2], but there are still some outstanding issues that need to be
addressed:
* vm.memfd_noexec=2 shouldn't reject old-style memfd_create(2) syscalls
because it will make it far to difficult to ever migrate. Instead it
should imply MFD_EXEC.
* The dmesg warnings are pr_warn_once(), which on most systems means
that they will be used up by systemd or some other boot process and
userspace developers will never see it.
- For the !(flags & (MFD_EXEC | MFD_NOEXEC_SEAL)) case, outputting a
rate-limited message to the kernel log is necessary to tell
userspace that they should add the new flags.
Arguably the most ideal way to deal with the spam concern[3,4]
while still prompting userspace to switch to the new flags would be
to only log the warning once per task or something similar.
However, adding something to task_struct for tracking this would be
needless bloat for a single pr_warn_ratelimited().
So just switch to pr_info_ratelimited() to avoid spamming the log
with something that isn't a real warning. There's lots of
info-level stuff in dmesg, it seems really unlikely that this
should be an actual problem. Most programs are already switching to
the new flags anyway.
- For the vm.memfd_noexec=2 case, we need to log a warning for every
failure because otherwise userspace will have no idea why their
previously working program started returning -EACCES (previously
-EINVAL) from memfd_create(2). pr_warn_once() is simply wrong here.
* The racheting mechanism for vm.memfd_noexec makes it incredibly
unappealing for most users to enable the sysctl because enabling it
on &init_pid_ns means you need a system reboot to unset it. Given the
actual security threat being protected against, CAP_SYS_ADMIN users
being restricted in this way makes little sense.
The argument for this ratcheting by the original author was that it
allows you to have a hierarchical setting that cannot be unset by
child pidnses, but this is not accurate -- changing the parent
pidns's vm.memfd_noexec setting to be more restrictive didn't affect
children.
Instead, switch the vm.memfd_noexec sysctl to be properly
hierarchical and allow CAP_SYS_ADMIN users (in the pidns's owning
userns) to lower the setting as long as it is not lower than the
parent's effective setting. This change also makes it so that
changing a parent pidns's vm.memfd_noexec will affect all
descendants, providing a properly hierarchical setting. The
performance impact of this is incredibly minimal since the maximum
depth of pidns is 32 and it is only checked during memfd_create(2)
and unshare(CLONE_NEWPID).
* The memfd selftests would not exit with a non-zero error code when
certain tests that ran in a forked process (specifically the ones
related to MFD_EXEC and MFD_NOEXEC_SEAL) failed.
[1]: https://lore.kernel.org/all/ZJwcsU0vI-nzgOB_@codewreck.org/
[2]: https://lore.kernel.org/all/20230705063315.3680666-1-jeffxu@google.com/
[3]: https://lore.kernel.org/Y5yS8wCnuYGLHMj4@x1n/
[4]: https://lore.kernel.org/f185bb42-b29c-977e-312e-3349eea15383@linuxfoundatio…
Signed-off-by: Aleksa Sarai <cyphar(a)cyphar.com>
---
Changes in v2:
- Make vm.memfd_noexec restrictions properly hierarchical.
- Allow vm.memfd_noexec setting to be lowered by CAP_SYS_ADMIN as long
as it is not lower than the parent's effective setting.
- Fix the logging behaviour related to the new flags and
vm.memfd_noexec=2.
- Add more thorough tests for vm.memfd_noexec in selftests.
- v1: <https://lore.kernel.org/r/20230713143406.14342-1-cyphar@cyphar.com>
---
Aleksa Sarai (5):
selftests: memfd: error out test process when child test fails
memfd: do not -EACCES old memfd_create() users with vm.memfd_noexec=2
memfd: improve userspace warnings for missing exec-related flags
memfd: replace ratcheting feature from vm.memfd_noexec with hierarchy
selftests: improve vm.memfd_noexec sysctl tests
include/linux/pid_namespace.h | 39 ++--
kernel/pid.c | 3 +
kernel/pid_namespace.c | 6 +-
kernel/pid_sysctl.h | 28 ++-
mm/memfd.c | 33 ++-
tools/testing/selftests/memfd/memfd_test.c | 332 +++++++++++++++++++++++------
6 files changed, 322 insertions(+), 119 deletions(-)
---
base-commit: 3ff995246e801ea4de0a30860a1d8da4aeb538e7
change-id: 20230803-memfd-vm-noexec-uapi-fixes-ace725c67b0f
Best regards,
--
Aleksa Sarai <cyphar(a)cyphar.com>
Hi all,
Recently "memfd: improve userspace warnings for missing exec-related
flags" was merged. On my system, this is a regression, not an
improvement, because the entire 256k kernel log buffer (default on x86)
is filled with these warnings and "__do_sys_memfd_create: 122 callbacks
suppressed". I haven't investigated too closely, but the most likely
cause is Wayland libraries.
This is too serious of a consequence for using an old API, especially
considering how recently the flags were added. The vast majority of
software has not had time to add the flags: glibc does not define the
macros until 2.38 which was released less than one month ago, man-pages
does not document the flags, and according to Debian Code Search, only
systemd, stress-ng, and strace actually pass either of these flags.
Furthermore, since old kernels reject unknown flags, it's not just a
matter of defining and passing the flag; every program needs to
add logic to handle EINVAL and try again.
Some other way needs to be found to encourage userspace to add the
flags; otherwise, this message will be patched out because the kernel
log becomes unusable after running unupdated programs, which will still
exist even after upstreams are fixed. In particular, AppImages,
flatpaks, snaps, and similar app bundles contain vendored Wayland
libraries which can be difficult or impossible to update.
Thanks,
Alex.
This change introduces a new fcntl to check if an fd points to a memfd's
original open fd (the one created by memfd_create).
We encountered an issue with migrating memfds in CRIU (checkpoint
restore in userspace - it migrates running processes between
machines). Imagine a scenario:
1. Create a memfd. By default it's open with O_RDWR and yet one can
exec() to it (unlike with regular files, where one would get ETXTBSY).
2. Reopen that memfd with O_RDWR via /proc/self/fd/<fd>.
Now those 2 fds are indistinguishable from userspace. You can't exec()
to either of them (since the reopen incremented inode->i_writecount)
and their /proc/self/fdinfo/ are exactly the same. Unfortunately they
are not the same. If you close the second one, the first one becomes
exec()able again. If you close the first one, the other doesn't become
exec()able. Therefore during migration it does matter which is recreated
first and which is reopened but there is no way for CRIU to tell which
was first.
Michal Clapinski (2):
fcntl: add fcntl(F_CHECK_ORIGINAL_MEMFD)
selftests: test fcntl(F_CHECK_ORIGINAL_MEMFD)
fs/fcntl.c | 3 ++
include/uapi/linux/fcntl.h | 9 ++++++
tools/testing/selftests/memfd/memfd_test.c | 32 ++++++++++++++++++++++
3 files changed, 44 insertions(+)
--
2.42.0.283.g2d96d420d3-goog
Hi, Willy
Since we have already finished the size inflate regression task [1], to share
and discuss the progress about the -ENOSYS return work, here launchs a new
thread, it is split from [2].
[1]: https://lore.kernel.org/lkml/ZNtszQeigYuItaKA@1wt.eu/
[2]: https://lore.kernel.org/lkml/20230814172233.225944-1-falcon@tinylab.org/#R
This is only for brain storming, it is far from a solution ;-)
>
> > [...]
> > > >
> > > > /* __systry2() is used to select one of two provided low level syscalls */
> > > > #define __systry2(a, sys_a, sys_b) \
> > > > ((NOLIBC__NR_##a != NOLIBC__NR_NOSYS) ? (sys_a) : (sys_b))
> > >
> > > But this supposes that all of them are manually defined as you did above.
> > > I'd rather implement an ugly is_numeric() macro based on argument
> > > resolution. I've done it once in another project, I don't remember
> > > precisely where it is but I vaguely remember that it used to check
> > > that the string resolution of the argument gave a letter (when it
> > > does not exist) or a digit (when it does). I can look into that later
> > > if needed. But please avoid extra macro definitions as much as possible,
> > > they're a real pain to handle in the code. There's no error when one is
> > > missing or has a typo, it's difficult to follow them and they don't
> > > appear in the debugger.
> > >
> >
> > Yeah, your reply inspired me to look into the IS_ENABLED() from
> > ../include/linux/kconfig.h macro again, there was a __is_defined() there, let's
> > throw away the ugly sysnr.h. I thought of IS_ENABLED() was only for y/n/m
> > before, but it does return 0 when the macro is not defined, it uses the same
> > trick in syscall() to calculate the number of arguments, if the macro is not
> > defined, then, 0 "argument".
> >
>
> The above trick is only for ""#define something 1" ;-)
>
Here shares a little progress on this, I have found it is easy to implement an
ugly is_numeric() like macro as following:
/* Imported from include/linux/stringify.h */
#define __stringify_1(x...) #x
#define __stringify(x...) __stringify_1(x)
/*
* Check __NR_* definition by stringizing
*
* - The stringizing is to silence compile error about undefined macro
* - If defined, the result looks like "3", "(4000 + 168)", not begin with '_'
* - If not defined, the result looks like "__NR_read", begins with '_'
*/
#define __is_nr_defined(nr) ___is_nr_defined(__stringify(nr))
#define ___is_nr_defined(str) (str[0] != '_')
__is_nr_defined() is able to check if __NR_xxx is defined, but the harder part
is getting the number of defined __NR_* without the error about undefined
macro.
Of course, we can also use the __stringify() trick to do so, but it is
expensive (bigger size, worse performance) to unstringify and get the number
again, the expensive atoi() 'works' for the numeric __NR_*, but not work for
(__NR_*_base + offset) like __NR_* definitions (used by ARM and MIPS), a simple
interpreter is required for such cases and it is more expensive than atoi().
/* not for ARM and MIPS */
static int atoi(const char *s);
#define __get_nr(name) __nr_atoi(__stringify(__NR_##name))
#define __nr_atoi(str) (str[0] == '_' ? -1L : ___nr_atoi(str))
#define ___nr_atoi(str) (str[0] == '(' ? -1L : atoi(str))
Welcome more discussion or let's simply throw away this direction ;-)
But it may really help us to drop tons of duplicated code pieces like this:
#ifdef __NR_xxxx
...
#else
return -ENOSYS;
#endif
David, Thomas and Arnd, any inspiration on this, or is this really impossible
(or make things worse) in language level? ;-)
What I'm thinking about is something like this or similar (As Willy commented
before, the __sysdef() itself is not that good, please ignore itself, the core
target here is using a single -ENOSYS return for all of the undefined
branches):
#define __sysdef(name, ...) \
(__is_nr_defined(__NR_##name) ? my_syscall(__get_nr(name), ##__VA_ARGS__) : (long)-ENOSYS)
Or as Arnd replied in an old email thread before, perhaps the whole #ifdef's
code piece (and even the input types and return types of sys_*) above can be
generated from .tbl or the generic unistd.h automatically in the sysroot
installation stage?
BR,
Zhangjin
The state handle in kunit_module_notify() is not correct when
the mod->state switch from MODULE_STATE_COMING to MODULE_STATE_GOING.
And it's necessary to check NULL for kzalloc() in
kunit_parse_glob_filter().
The order in which memory is released in err path in kunit_filter_suites()
is also problematic.
And there is a possible memory leak in kunit_filter_suites().
This patchset fix the above issues.
Jinjie Ruan (4):
kunit: Fix wild-memory-access bug in kunit_free_suite_set()
kunit: Fix possible null-ptr-deref in kunit_parse_glob_filter()
kunit: Fix possible memory leak in kunit_filter_suites()
kunit: Fix the wrong error path in kunit_filter_suites()
lib/kunit/executor.c | 39 +++++++++++++++++++++++++++------------
lib/kunit/test.c | 3 ++-
2 files changed, 29 insertions(+), 13 deletions(-)
--
2.34.1
This patch chain changes the logging implementation to use string_stream
so that the log will grow dynamically.
The first 8 patches add test code for string_stream, and make some
changes to string_stream needed to be able to use it for the log.
The final patch adds a performance report of string_stream.
CHANGES SINCE V5:
Patch 2:
- Avoid cast warning when using KUNIT_EXPECT_EQ() on a gfp_t. Instead pass
the result of the comparison to KUNIT_EXPECT_TRUE(). While it would be
nice to use KUNIT_EXPECT_EQ(), it's probably better to avoid introducing
build or sparse warnings.
- In string_stream_append_test() rename original_content to
stream1_content_before_append.
Patch 7:
- Make string_stream_clear() public (in v5 this was done in patch #8).
- In string-stream-test.c add a wrapper for kfree() to prevent a cast
warning when calling kunit_add_action().
Patch 8:
- Fix memory leak when calling the redirected string_stream_destroy_stub().
Patch 9:
- In kunit-test.c: add wrapper function around kfree() to prevent cast
warning when calling kunit_add_action().
- Fix unused variable warning in kunit_log_test() when built as a module.
Richard Fitzgerald (10):
kunit: string-stream: Don't create a fragment for empty strings
kunit: string-stream: Improve testing of string_stream
kunit: string-stream: Add option to make all lines end with newline
kunit: string-stream-test: Add cases for string_stream newline
appending
kunit: Don't use a managed alloc in is_literal()
kunit: string-stream: Add kunit_alloc_string_stream()
kunit: string-stream: Decouple string_stream from kunit
kunit: string-stream: Add tests for freeing resource-managed
string_stream
kunit: Use string_stream for test log
kunit: string-stream: Test performance of string_stream
include/kunit/test.h | 14 +-
lib/kunit/assert.c | 14 +-
lib/kunit/debugfs.c | 36 ++-
lib/kunit/kunit-test.c | 56 +++-
lib/kunit/string-stream-test.c | 525 +++++++++++++++++++++++++++++++--
lib/kunit/string-stream.c | 100 +++++--
lib/kunit/string-stream.h | 16 +-
lib/kunit/test.c | 50 +---
8 files changed, 688 insertions(+), 123 deletions(-)
--
2.30.2
The benchmark command handling (-b) in resctrl selftests is overly
complicated code. This series turns the benchmark command immutable to
preserve it for all selftests and improves benchmark command related
error handling.
This series also ends up removing the strcpy() calls which were pointed
out earlier.
v4:
- Correct off-by-one error in -b processing
- Reordered code in main() to make freeing span_str simpler (in new patch)
- Use consistent style for const char * const *
v3:
- Removed DEFAULT_SPAN_STR for real and the duplicated copy of defines
that made to v2 likely due to my incorrect conflict resolutions
v2:
- Added argument length check into patch 1/7
- Updated also -b line in help message.
- Document -b argument related "algorithm"
- Use asprintf() to convert defined constant int to string
- Improved changelog texts
- Added \n to ksft_exit_fail_msg() call messages.
- Print DEFAULT_SPAN with %u instead of %zu to avoid need to cast it
Ilpo Järvinen (8):
selftests/resctrl: Ensure the benchmark commands fits to its array
selftests/resctrl: Correct benchmark command help
selftests/resctrl: Remove bw_report and bm_type from main()
selftests/resctrl: Simplify span lifetime
selftests/resctrl: Reorder resctrl FS prep code and benchmark_cmd init
selftests/resctrl: Make benchmark command const and build it with
pointers
selftests/resctrl: Remove ben_count variable
selftests/resctrl: Cleanup benchmark argument parsing
tools/testing/selftests/resctrl/cache.c | 5 +-
tools/testing/selftests/resctrl/cat_test.c | 13 +--
tools/testing/selftests/resctrl/cmt_test.c | 34 ++++--
tools/testing/selftests/resctrl/mba_test.c | 4 +-
tools/testing/selftests/resctrl/mbm_test.c | 7 +-
tools/testing/selftests/resctrl/resctrl.h | 16 +--
.../testing/selftests/resctrl/resctrl_tests.c | 100 ++++++++----------
tools/testing/selftests/resctrl/resctrl_val.c | 10 +-
8 files changed, 104 insertions(+), 85 deletions(-)
--
2.30.2