* [LTP] [PATCH] fanotify: prepare tests for thread pidfd reporting
@ 2026-05-27 6:40 ` AnonymeMeow
0 siblings, 0 replies; 21+ messages in thread
From: AnonymeMeow @ 2026-05-27 6:40 UTC (permalink / raw)
To: amir73il, ltp; +Cc: jack, linux-kernel, AnonymeMeow, repnop, linux-fsdevel
On 2026-05-25 12:04:32+02:00, Amir Goldstein wrote:
> On Sun, May 24, 2026 at 12:25 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> > The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> > exclusive because by the time the pidfd support was introduced to
> > fanotify, pidfds could only be created for thread group leaders. Now
> > that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> > this restriction can be lifted.
> >
> > This patch allows listeners using FAN_REPORT_PIDFD | FAN_REPORT_TID
> > to receive the pidfd referring to the thread that triggered the
> > event.
> >
> > Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
>
> Regarding the legal standpoint, as far as I can tell, commit d4563201f33a0
> ("Documentation: simplify and clarify DCO contribution example language")
> explicitly allowed pseudonyms, so although unorthodox, this seems to be legit.
>
> In any case, the contributing itself seems legit to me, so
>
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
>
> But before merging this, AnonymeMeow, since it is your first contribution
> to the Linux kernel (right?) as well as a "I am not a robot" challenge
> I would like to first see that the tests have been dealt with.
>
> The LTP test fanotify20 explicitly checks that this flag combination is not
> allowed. Need to discuss an acceptable solution with LTP developers -
> remove this check depending on kernel version or whatever they choose.
>
> The LTP test fanotify21 tests the FAN_REPORT_PIDFD functionality.
> Please add two new variants for REPORT_TID:
> .test_variants = 4,
>
> #define TST_VARIANT_FD_ERROR (tst_variant & 1)
> #define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
>
> The existing checks if (tst_variant) become if (TST_VARIANT_FD_ERROR)
> so bit 0 represents the FAN_REPORT_FD_ERROR variation.
>
> in do_setup() need to use if (TST_VARIANT_PIDFD_THREAD)
> to add FAN_REPORT_TID and to use PIDFD_THREAD for pidfd.
>
> Follow the footsteps of fd_error_unsupported to skip the
> TST_VARIANT_PIDFD_THREAD variants on kernels where the flag
> combination is not supported.
>
> Good luck,
> Amir.
Thanks for the helpful and detailed guidance on how to extend the tests!
I've updated the test cases according to your suggestions.
After applying my previous patch, the LTP fanotify20 test needs to probe
whether the running kernel supports thread pidfd reporting. I didn't use
kernel version check because the LTP documentation notes that kernel
versions do not always correspond to a well-defined feature set due to
distribution backports, and recommends runtime feature checks. I probe
whether fanotify_init() succeeds with FAN_REPORT_PIDFD | FAN_REPORT_TID
as a workaround. The first test case uses exactly the same flags, but it
still verifies the return value and errno, so I am not sure whether this
should be considered duplicate logic. I also added one test case that
passes all relevant FAN_REPORT_* flags and verifies whether
fanotify_init() behaves as expected. The expected result depends on the
previous probe result.
Also, I added one test case and two test variants to LTP test
fanotify21. The new test case verifies that fanotify reports the correct
pidfd for the event generated by a child process or a worker thread
created by the main thread. This is especially useful for the thread
pidfd test variants, because it checks that the pidfd reported by
fanotify really refers to the event-generating thread rather than to
the thread-group leader. The two new test variants extend the existing
ones by adding the thread pidfd mode, as you suggested.
I ran all fanotify-related LTP tests on both a vanilla Linux 7.1-rc5
kernel and a Linux 7.1-rc5 kernel with my patch applied. Both kernels
passed all the tests. They were both built with the config file from the
official Arch Linux package repo. More specifically, both kernels passed
all three test cases in fanotify20, with no skips. The test cases expect
different results from the two kernels, which is the expected behavior.
For fanotify21, the patched kernel passed all test cases in all test
variants. On the unpatched kernel, all test cases passed for variants
without TST_VARIANT_PIDFD_THREAD, while all test cases in variants with
TST_VARIANT_PIDFD_THREAD were skipped, which is also the expected
behavior.
Again, thanks for the detailed guidance on this!
With Best Regards,
AnonymeMeow
Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
testcases/kernel/syscalls/fanotify/Makefile | 2 +-
.../kernel/syscalls/fanotify/fanotify20.c | 24 +-
.../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
3 files changed, 184 insertions(+), 66 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
index 3628094ba..b20bb50e9 100644
--- a/testcases/kernel/syscalls/fanotify/Makefile
+++ b/testcases/kernel/syscalls/fanotify/Makefile
@@ -2,7 +2,7 @@
# Copyright (c) Jan Kara <jack@suse.cz>, 2013
top_srcdir ?= ../../../..
-fanotify11: CFLAGS+=-pthread
+fanotify11 fanotify21: CFLAGS+=-pthread
include $(top_srcdir)/include/mk/testcases.mk
include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index b32ecf6aa..5d77b485c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -28,19 +28,27 @@
#define FLAGS_DESC(x) .flags = x, .desc = #x
static int fd;
+static int thread_pidfd_unsupported;
static struct test_case_t {
unsigned int flags;
char *desc;
int exp_errno;
+ unsigned int needs_thread_pidfd;
} test_cases[] = {
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
.exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
},
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
},
+ {
+ FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
+ .exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
+ },
};
static void do_setup(void)
@@ -51,17 +59,29 @@ static void do_setup(void)
*/
REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
MOUNT_PATH);
+
+ /*
+ * Check whether the kernel supports FAN_REPORT_PIDFD in combination
+ * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
+ * set expect different errno values depending on whether this
+ * combination is supported.
+ */
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
}
static void do_test(unsigned int i)
{
struct test_case_t *tc = &test_cases[i];
- tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
+ int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
+ 0 : tc->exp_errno;
+
+ tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
tc->desc);
TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
- tc->exp_errno);
+ exp_errno);
if (fd > 0)
SAFE_CLOSE(fd);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 340fb0018..4629860da 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -20,9 +20,11 @@
#include <stdlib.h>
#include <string.h>
#include <sys/mount.h>
+#include <pthread.h>
#include "tst_test.h"
#include "tst_safe_stdio.h"
#include "tst_safe_macros.h"
+#include "tst_safe_pthread.h"
#include "lapi/pidfd.h"
#ifdef HAVE_SYS_FANOTIFY_H
@@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
static struct test_case_t {
char *name;
- int fork;
+ int trigger_in_child;
int want_pidfd_err;
int remount_ro;
} test_cases[] = {
@@ -52,6 +54,12 @@ static struct test_case_t {
0,
0,
},
+ {
+ "return a valid pidfd for event created by child",
+ 1,
+ 0,
+ 0,
+ },
{
"return invalid pidfd for event created by terminated child",
1,
@@ -68,16 +76,17 @@ static struct test_case_t {
static int fanotify_fd;
static char event_buf[BUF_SZ];
-static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
+static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
static int fd_error_unsupported;
+static int thread_pidfd_unsupported;
+
+#define TST_VARIANT_FD_ERROR (tst_variant & 1)
+#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
-static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
+static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
{
char *fdinfo_path;
- struct pidfd_fdinfo_t *pidfd_fdinfo;
-
- pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
@@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
free(fdinfo_path);
-
- return pidfd_fdinfo;
}
static void generate_event(void)
@@ -100,30 +107,91 @@ static void generate_event(void)
SAFE_CLOSE(fd);
}
-static void do_fork(void)
+static pid_t do_fork(int want_pidfd_err)
{
- int status;
+ int pidfd;
pid_t child;
child = SAFE_FORK();
if (child == 0) {
SAFE_CLOSE(fanotify_fd);
generate_event();
+ TST_CHECKPOINT_WAIT(0);
exit(EXIT_SUCCESS);
}
- SAFE_WAITPID(child, &status, 0);
- if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
- tst_brk(TBROK,
- "child process terminated incorrectly");
+ pidfd = SAFE_PIDFD_OPEN(child, 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
+ if (want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_WAITPID(child, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+
+ return -1;
+ }
+
+ return child;
}
-static void do_setup(void)
+static void *thread_generate_event(void *arg)
+{
+ *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
+ TST_CHECKPOINT_WAKE(0);
+
+ generate_event();
+ TST_CHECKPOINT_WAIT(0);
+ pthread_exit(0);
+}
+
+static pthread_t do_pthread_create(int want_pidfd_err)
{
int pidfd;
+ pthread_t worker;
+
+ SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
+
+ TST_CHECKPOINT_WAIT(0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+
+ if (want_pidfd_err) {
+ int status;
+ struct pidfd_fdinfo_t thread_pidfd_fdinfo;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_PTHREAD_JOIN(worker, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+
+ /*
+ * Unlike waitpid(), pthread_join() only waits until the worker thread
+ * has exited from the pthread point of view. The thread may still be
+ * visible through its pidfd for a short time afterwards, and fanotify
+ * creates the event pidfd when the event is read. Wait until the
+ * worker pidfd fdinfo reports Pid: -1 before reading the event so
+ * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
+ */
+ do {
+ read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
+ } while (thread_pidfd_fdinfo.pid != -1);
+
+ SAFE_CLOSE(pidfd);
+
+ return -1;
+ }
+
+ SAFE_CLOSE(pidfd);
+
+ return worker;
+}
+
+static void do_setup(void)
+{
int init_flags = FAN_REPORT_PIDFD;
- if (tst_variant) {
+ if (TST_VARIANT_FD_ERROR) {
fanotify_fd = -1;
fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
if (fd_error_unsupported)
@@ -131,6 +199,15 @@ static void do_setup(void)
init_flags |= FAN_REPORT_FD_ERROR;
}
+ if (TST_VARIANT_PIDFD_THREAD) {
+ fanotify_fd = -1;
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
+ if (thread_pidfd_unsupported)
+ return;
+ init_flags |= FAN_REPORT_TID;
+ }
+
SAFE_TOUCH(TEST_FILE, 0666, NULL);
/*
@@ -144,15 +221,6 @@ static void do_setup(void)
fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
TEST_FILE);
-
- pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
-
- self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
- if (self_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "pidfd=%d, failed to read pidfd fdinfo",
- pidfd);
- }
}
static void do_test(unsigned int num)
@@ -160,17 +228,29 @@ static void do_test(unsigned int num)
int i = 0, len;
struct test_case_t *tc = &test_cases[num];
int nopidfd_err = tc->want_pidfd_err ?
- (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
- int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
+ (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
+ int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
+ union {
+ pid_t pid;
+ pthread_t pthread_id;
+ } worker_id;
tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
- tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
+ TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
+ "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
+ (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
- if (fd_error_unsupported && tst_variant) {
+ if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
return;
}
+ if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
+ FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
+ thread_pidfd_unsupported);
+ return;
+ }
+
if (tc->remount_ro) {
/* SAFE_MOUNT fails to remount FUSE */
if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
@@ -182,14 +262,30 @@ static void do_test(unsigned int num)
}
/*
- * Generate the event in either self or a child process. Event
- * generation in a child process is done so that the FAN_NOPIDFD case
- * can be verified.
+ * Generate the event either in the current task or in another task.
+ * When trigger_in_child is set, the event can be generated by either
+ * a child process or a worker thread depending on the test variant.
+ * The want_pidfd_err field determines whether the event-generating
+ * task is still valid when the event is read.
*/
- if (tc->fork)
- do_fork();
- else
+ if (tc->trigger_in_child) {
+ if (TST_VARIANT_PIDFD_THREAD)
+ worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
+ else
+ worker_id.pid = do_fork(tc->want_pidfd_err);
+ } else {
+ /*
+ * Although the expected pid and the pid reported by fanotify are
+ * the same in this case, pidfds created with and without PIDFD_THREAD
+ * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
+ * pidfd fdinfo so that the fdinfo can be compared bitwise.
+ */
+ int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
generate_event();
+ }
/*
* Read all of the queued events into the provided event
@@ -208,7 +304,7 @@ static void do_test(unsigned int num)
while (i < len) {
struct fanotify_event_metadata *event;
struct fanotify_event_info_pidfd *info;
- struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
+ struct pidfd_fdinfo_t event_pidfd_fdinfo;
event = (struct fanotify_event_metadata *)&event_buf[i];
info = (struct fanotify_event_info_pidfd *)(event + 1);
@@ -288,39 +384,32 @@ static void do_test(unsigned int num)
* No pidfd errors occurred, continue with verifying pidfd
* fdinfo validity.
*/
- event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
- if (event_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "reading fdinfo for pidfd: %d "
- "describing pid: %u failed",
- info->pidfd,
- (unsigned int)event->pid);
- goto next_event;
- } else if (event_pidfd_fdinfo->pid != event->pid) {
+ read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
+ if (event_pidfd_fdinfo.pid != event->pid) {
tst_res(TFAIL,
"pidfd provided for incorrect pid "
"(expected pidfd for pid: %u, got pidfd for "
"pid: %u)",
(unsigned int)event->pid,
- (unsigned int)event_pidfd_fdinfo->pid);
+ (unsigned int)event_pidfd_fdinfo.pid);
goto next_event;
- } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
+ } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
sizeof(struct pidfd_fdinfo_t))) {
tst_res(TFAIL,
"pidfd fdinfo values for self and event differ "
"(expected pos: %d, flags: %x, mnt_id: %d, "
"pid: %d, ns_pid: %d, got pos: %d, "
"flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
- self_pidfd_fdinfo->pos,
- self_pidfd_fdinfo->flags,
- self_pidfd_fdinfo->mnt_id,
- self_pidfd_fdinfo->pid,
- self_pidfd_fdinfo->ns_pid,
- event_pidfd_fdinfo->pos,
- event_pidfd_fdinfo->flags,
- event_pidfd_fdinfo->mnt_id,
- event_pidfd_fdinfo->pid,
- event_pidfd_fdinfo->ns_pid);
+ expected_pidfd_fdinfo.pos,
+ expected_pidfd_fdinfo.flags,
+ expected_pidfd_fdinfo.mnt_id,
+ expected_pidfd_fdinfo.pid,
+ expected_pidfd_fdinfo.ns_pid,
+ event_pidfd_fdinfo.pos,
+ event_pidfd_fdinfo.flags,
+ event_pidfd_fdinfo.mnt_id,
+ event_pidfd_fdinfo.pid,
+ event_pidfd_fdinfo.ns_pid);
goto next_event;
} else {
tst_res(TPASS,
@@ -342,9 +431,20 @@ next_event:
if (info && info->pidfd >= 0)
SAFE_CLOSE(info->pidfd);
+ }
- if (event_pidfd_fdinfo)
- free(event_pidfd_fdinfo);
+ if (tc->trigger_in_child && !tc->want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ if (TST_VARIANT_PIDFD_THREAD) {
+ SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+ } else {
+ SAFE_WAITPID(worker_id.pid, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+ }
}
}
@@ -352,19 +452,17 @@ static void do_cleanup(void)
{
if (fanotify_fd >= 0)
SAFE_CLOSE(fanotify_fd);
-
- if (self_pidfd_fdinfo)
- free(self_pidfd_fdinfo);
}
static struct tst_test test = {
.setup = do_setup,
.test = do_test,
.tcnt = ARRAY_SIZE(test_cases),
- .test_variants = 2,
+ .test_variants = 4,
.cleanup = do_cleanup,
.all_filesystems = 1,
.needs_root = 1,
+ .needs_checkpoints = 1,
.mount_device = 1,
.mntpoint = MOUNT_PATH,
.forks_child = 1,
--
2.54.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH] fanotify: prepare tests for thread pidfd reporting
2026-05-27 6:40 ` [LTP] " AnonymeMeow
@ 2026-05-27 7:23 ` Petr Vorel
-1 siblings, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2026-05-27 7:23 UTC (permalink / raw)
To: AnonymeMeow; +Cc: amir73il, ltp, jack, repnop, linux-fsdevel, linux-kernel
Hi AnonymeMeow, Amir,
[ Cc Cyril ]
> On 2026-05-25 12:04:32+02:00, Amir Goldstein wrote:
> > On Sun, May 24, 2026 at 12:25 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
@AnonymeMeow nit: if you put whole conversation after --- (below), it will not
be part of the commit message when one applies the patch. And here should be a
proper commit message.
FYI We have just 2 days before releasing new LTP version, this will not get in
anyway. At least I'm not sure if we want to risk regression in the release for
functionality which is not yet even in the next tree.
Also I found that fanotify21.c on the current master fails when running more
than 1 iteration:
# ./fanotify21 -i2
...
tst_test.c:1980: TINFO: === Testing on ext2 ===
tst_test.c:1291: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.4 (6-Mar-2025)
tst_test.c:1303: TINFO: Mounting /dev/loop0 to /tmp/LTP_fanWZr7p3/fs_mnt fstyp=ext2 flags=0
fanotify21.c:238: TINFO: Test #0.0: return a valid pidfd for event created by self
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:415: TPASS: got an event with a valid pidfd info record, mask: 32, pid: 234527, fd: 4, pidfd: 5, info_type: 4, info_len: 8
fanotify21.c:238: TINFO: Test #1.0: return a valid pidfd for event created by child
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:415: TPASS: got an event with a valid pidfd info record, mask: 32, pid: 234528, fd: 4, pidfd: 5, info_type: 4, info_len: 8
fanotify21.c:238: TINFO: Test #2.0: return invalid pidfd for event created by terminated child
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:375: TPASS: pid: 234529 terminated before pidfd was created, pidfd set to the value of: -1, as expected
fanotify21.c:238: TINFO: Test #3.0: fail to open rw fd for event created on read-only mount
fanotify21.c:297: TPASS: cannot read event with rw fd from a ro fs
fanotify21.c:238: TINFO: Test #0.0: return a valid pidfd for event created by self
fanotify21.c:300: TBROK: reading fanotify events failed: EROFS (30)
Summary:
passed 7
failed 0
broken 1
skipped 0
warnings 0
That should be fixed. It might be worth to fix that before the release.
Kind regards,
Petr
> > > The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> > > exclusive because by the time the pidfd support was introduced to
> > > fanotify, pidfds could only be created for thread group leaders. Now
> > > that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> > > this restriction can be lifted.
> > > This patch allows listeners using FAN_REPORT_PIDFD | FAN_REPORT_TID
> > > to receive the pidfd referring to the thread that triggered the
> > > event.
> > > Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> > Regarding the legal standpoint, as far as I can tell, commit d4563201f33a0
> > ("Documentation: simplify and clarify DCO contribution example language")
> > explicitly allowed pseudonyms, so although unorthodox, this seems to be legit.
> > In any case, the contributing itself seems legit to me, so
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > But before merging this, AnonymeMeow, since it is your first contribution
> > to the Linux kernel (right?) as well as a "I am not a robot" challenge
> > I would like to first see that the tests have been dealt with.
> > The LTP test fanotify20 explicitly checks that this flag combination is not
> > allowed. Need to discuss an acceptable solution with LTP developers -
> > remove this check depending on kernel version or whatever they choose.
> > The LTP test fanotify21 tests the FAN_REPORT_PIDFD functionality.
> > Please add two new variants for REPORT_TID:
> > .test_variants = 4,
> > #define TST_VARIANT_FD_ERROR (tst_variant & 1)
> > #define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
> > The existing checks if (tst_variant) become if (TST_VARIANT_FD_ERROR)
> > so bit 0 represents the FAN_REPORT_FD_ERROR variation.
> > in do_setup() need to use if (TST_VARIANT_PIDFD_THREAD)
> > to add FAN_REPORT_TID and to use PIDFD_THREAD for pidfd.
> > Follow the footsteps of fd_error_unsupported to skip the
> > TST_VARIANT_PIDFD_THREAD variants on kernels where the flag
> > combination is not supported.
> > Good luck,
> > Amir.
> Thanks for the helpful and detailed guidance on how to extend the tests!
> I've updated the test cases according to your suggestions.
> After applying my previous patch, the LTP fanotify20 test needs to probe
> whether the running kernel supports thread pidfd reporting. I didn't use
> kernel version check because the LTP documentation notes that kernel
> versions do not always correspond to a well-defined feature set due to
> distribution backports, and recommends runtime feature checks. I probe
> whether fanotify_init() succeeds with FAN_REPORT_PIDFD | FAN_REPORT_TID
> as a workaround. The first test case uses exactly the same flags, but it
> still verifies the return value and errno, so I am not sure whether this
> should be considered duplicate logic. I also added one test case that
> passes all relevant FAN_REPORT_* flags and verifies whether
> fanotify_init() behaves as expected. The expected result depends on the
> previous probe result.
> Also, I added one test case and two test variants to LTP test
> fanotify21. The new test case verifies that fanotify reports the correct
> pidfd for the event generated by a child process or a worker thread
> created by the main thread. This is especially useful for the thread
> pidfd test variants, because it checks that the pidfd reported by
> fanotify really refers to the event-generating thread rather than to
> the thread-group leader. The two new test variants extend the existing
> ones by adding the thread pidfd mode, as you suggested.
> I ran all fanotify-related LTP tests on both a vanilla Linux 7.1-rc5
> kernel and a Linux 7.1-rc5 kernel with my patch applied. Both kernels
> passed all the tests. They were both built with the config file from the
> official Arch Linux package repo. More specifically, both kernels passed
> all three test cases in fanotify20, with no skips. The test cases expect
> different results from the two kernels, which is the expected behavior.
> For fanotify21, the patched kernel passed all test cases in all test
> variants. On the unpatched kernel, all test cases passed for variants
> without TST_VARIANT_PIDFD_THREAD, while all test cases in variants with
> TST_VARIANT_PIDFD_THREAD were skipped, which is also the expected
> behavior.
> Again, thanks for the detailed guidance on this!
> With Best Regards,
> AnonymeMeow
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
> testcases/kernel/syscalls/fanotify/Makefile | 2 +-
> .../kernel/syscalls/fanotify/fanotify20.c | 24 +-
> .../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
> 3 files changed, 184 insertions(+), 66 deletions(-)
> diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
> index 3628094ba..b20bb50e9 100644
> --- a/testcases/kernel/syscalls/fanotify/Makefile
> +++ b/testcases/kernel/syscalls/fanotify/Makefile
> @@ -2,7 +2,7 @@
> # Copyright (c) Jan Kara <jack@suse.cz>, 2013
> top_srcdir ?= ../../../..
> -fanotify11: CFLAGS+=-pthread
> +fanotify11 fanotify21: CFLAGS+=-pthread
> include $(top_srcdir)/include/mk/testcases.mk
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index b32ecf6aa..5d77b485c 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -28,19 +28,27 @@
> #define FLAGS_DESC(x) .flags = x, .desc = #x
> static int fd;
> +static int thread_pidfd_unsupported;
> static struct test_case_t {
> unsigned int flags;
> char *desc;
> int exp_errno;
> + unsigned int needs_thread_pidfd;
> } test_cases[] = {
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
> .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> },
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> },
> + {
> + FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> + .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> + },
> };
> static void do_setup(void)
> @@ -51,17 +59,29 @@ static void do_setup(void)
> */
> REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
> MOUNT_PATH);
> +
> + /*
> + * Check whether the kernel supports FAN_REPORT_PIDFD in combination
> + * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
> + * set expect different errno values depending on whether this
> + * combination is supported.
> + */
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
> }
> static void do_test(unsigned int i)
> {
> struct test_case_t *tc = &test_cases[i];
> - tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
> + int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
> + 0 : tc->exp_errno;
> +
> + tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
> tc->desc);
> TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
> - tc->exp_errno);
> + exp_errno);
> if (fd > 0)
> SAFE_CLOSE(fd);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> index 340fb0018..4629860da 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify21.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -20,9 +20,11 @@
> #include <stdlib.h>
> #include <string.h>
> #include <sys/mount.h>
> +#include <pthread.h>
> #include "tst_test.h"
> #include "tst_safe_stdio.h"
> #include "tst_safe_macros.h"
> +#include "tst_safe_pthread.h"
> #include "lapi/pidfd.h"
> #ifdef HAVE_SYS_FANOTIFY_H
> @@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
> static struct test_case_t {
> char *name;
> - int fork;
> + int trigger_in_child;
> int want_pidfd_err;
> int remount_ro;
> } test_cases[] = {
> @@ -52,6 +54,12 @@ static struct test_case_t {
> 0,
> 0,
> },
> + {
> + "return a valid pidfd for event created by child",
> + 1,
> + 0,
> + 0,
> + },
> {
> "return invalid pidfd for event created by terminated child",
> 1,
> @@ -68,16 +76,17 @@ static struct test_case_t {
> static int fanotify_fd;
> static char event_buf[BUF_SZ];
> -static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
> +static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
> static int fd_error_unsupported;
> +static int thread_pidfd_unsupported;
> +
> +#define TST_VARIANT_FD_ERROR (tst_variant & 1)
> +#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
> -static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> +static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
> {
> char *fdinfo_path;
> - struct pidfd_fdinfo_t *pidfd_fdinfo;
> -
> - pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
> SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
> SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
> @@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
> free(fdinfo_path);
> -
> - return pidfd_fdinfo;
> }
> static void generate_event(void)
> @@ -100,30 +107,91 @@ static void generate_event(void)
> SAFE_CLOSE(fd);
> }
> -static void do_fork(void)
> +static pid_t do_fork(int want_pidfd_err)
> {
> - int status;
> + int pidfd;
> pid_t child;
> child = SAFE_FORK();
> if (child == 0) {
> SAFE_CLOSE(fanotify_fd);
> generate_event();
> + TST_CHECKPOINT_WAIT(0);
> exit(EXIT_SUCCESS);
> }
> - SAFE_WAITPID(child, &status, 0);
> - if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> - tst_brk(TBROK,
> - "child process terminated incorrectly");
> + pidfd = SAFE_PIDFD_OPEN(child, 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> + if (want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_WAITPID(child, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> +
> + return -1;
> + }
> +
> + return child;
> }
> -static void do_setup(void)
> +static void *thread_generate_event(void *arg)
> +{
> + *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
> + TST_CHECKPOINT_WAKE(0);
> +
> + generate_event();
> + TST_CHECKPOINT_WAIT(0);
> + pthread_exit(0);
> +}
> +
> +static pthread_t do_pthread_create(int want_pidfd_err)
> {
> int pidfd;
> + pthread_t worker;
> +
> + SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
> +
> + TST_CHECKPOINT_WAIT(0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> +
> + if (want_pidfd_err) {
> + int status;
> + struct pidfd_fdinfo_t thread_pidfd_fdinfo;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_PTHREAD_JOIN(worker, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> +
> + /*
> + * Unlike waitpid(), pthread_join() only waits until the worker thread
> + * has exited from the pthread point of view. The thread may still be
> + * visible through its pidfd for a short time afterwards, and fanotify
> + * creates the event pidfd when the event is read. Wait until the
> + * worker pidfd fdinfo reports Pid: -1 before reading the event so
> + * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
> + */
> + do {
> + read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
> + } while (thread_pidfd_fdinfo.pid != -1);
> +
> + SAFE_CLOSE(pidfd);
> +
> + return -1;
> + }
> +
> + SAFE_CLOSE(pidfd);
> +
> + return worker;
> +}
> +
> +static void do_setup(void)
> +{
> int init_flags = FAN_REPORT_PIDFD;
> - if (tst_variant) {
> + if (TST_VARIANT_FD_ERROR) {
> fanotify_fd = -1;
> fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
> if (fd_error_unsupported)
> @@ -131,6 +199,15 @@ static void do_setup(void)
> init_flags |= FAN_REPORT_FD_ERROR;
> }
> + if (TST_VARIANT_PIDFD_THREAD) {
> + fanotify_fd = -1;
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
> + if (thread_pidfd_unsupported)
> + return;
> + init_flags |= FAN_REPORT_TID;
> + }
> +
> SAFE_TOUCH(TEST_FILE, 0666, NULL);
> /*
> @@ -144,15 +221,6 @@ static void do_setup(void)
> fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
> SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
> TEST_FILE);
> -
> - pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
> -
> - self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
> - if (self_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "pidfd=%d, failed to read pidfd fdinfo",
> - pidfd);
> - }
> }
> static void do_test(unsigned int num)
> @@ -160,17 +228,29 @@ static void do_test(unsigned int num)
> int i = 0, len;
> struct test_case_t *tc = &test_cases[num];
> int nopidfd_err = tc->want_pidfd_err ?
> - (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
> - int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
> + (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
> + int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
> + union {
> + pid_t pid;
> + pthread_t pthread_id;
> + } worker_id;
> tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
> - tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
> + TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
> + "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
> + (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
> - if (fd_error_unsupported && tst_variant) {
> + if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
> FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
> return;
> }
> + if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
> + FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
> + thread_pidfd_unsupported);
> + return;
> + }
> +
> if (tc->remount_ro) {
> /* SAFE_MOUNT fails to remount FUSE */
> if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> @@ -182,14 +262,30 @@ static void do_test(unsigned int num)
> }
> /*
> - * Generate the event in either self or a child process. Event
> - * generation in a child process is done so that the FAN_NOPIDFD case
> - * can be verified.
> + * Generate the event either in the current task or in another task.
> + * When trigger_in_child is set, the event can be generated by either
> + * a child process or a worker thread depending on the test variant.
> + * The want_pidfd_err field determines whether the event-generating
> + * task is still valid when the event is read.
> */
> - if (tc->fork)
> - do_fork();
> - else
> + if (tc->trigger_in_child) {
> + if (TST_VARIANT_PIDFD_THREAD)
> + worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
> + else
> + worker_id.pid = do_fork(tc->want_pidfd_err);
> + } else {
> + /*
> + * Although the expected pid and the pid reported by fanotify are
> + * the same in this case, pidfds created with and without PIDFD_THREAD
> + * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
> + * pidfd fdinfo so that the fdinfo can be compared bitwise.
> + */
> + int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> generate_event();
> + }
> /*
> * Read all of the queued events into the provided event
> @@ -208,7 +304,7 @@ static void do_test(unsigned int num)
> while (i < len) {
> struct fanotify_event_metadata *event;
> struct fanotify_event_info_pidfd *info;
> - struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
> + struct pidfd_fdinfo_t event_pidfd_fdinfo;
> event = (struct fanotify_event_metadata *)&event_buf[i];
> info = (struct fanotify_event_info_pidfd *)(event + 1);
> @@ -288,39 +384,32 @@ static void do_test(unsigned int num)
> * No pidfd errors occurred, continue with verifying pidfd
> * fdinfo validity.
> */
> - event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
> - if (event_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "reading fdinfo for pidfd: %d "
> - "describing pid: %u failed",
> - info->pidfd,
> - (unsigned int)event->pid);
> - goto next_event;
> - } else if (event_pidfd_fdinfo->pid != event->pid) {
> + read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
> + if (event_pidfd_fdinfo.pid != event->pid) {
> tst_res(TFAIL,
> "pidfd provided for incorrect pid "
> "(expected pidfd for pid: %u, got pidfd for "
> "pid: %u)",
> (unsigned int)event->pid,
> - (unsigned int)event_pidfd_fdinfo->pid);
> + (unsigned int)event_pidfd_fdinfo.pid);
> goto next_event;
> - } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
> + } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
> sizeof(struct pidfd_fdinfo_t))) {
> tst_res(TFAIL,
> "pidfd fdinfo values for self and event differ "
> "(expected pos: %d, flags: %x, mnt_id: %d, "
> "pid: %d, ns_pid: %d, got pos: %d, "
> "flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
> - self_pidfd_fdinfo->pos,
> - self_pidfd_fdinfo->flags,
> - self_pidfd_fdinfo->mnt_id,
> - self_pidfd_fdinfo->pid,
> - self_pidfd_fdinfo->ns_pid,
> - event_pidfd_fdinfo->pos,
> - event_pidfd_fdinfo->flags,
> - event_pidfd_fdinfo->mnt_id,
> - event_pidfd_fdinfo->pid,
> - event_pidfd_fdinfo->ns_pid);
> + expected_pidfd_fdinfo.pos,
> + expected_pidfd_fdinfo.flags,
> + expected_pidfd_fdinfo.mnt_id,
> + expected_pidfd_fdinfo.pid,
> + expected_pidfd_fdinfo.ns_pid,
> + event_pidfd_fdinfo.pos,
> + event_pidfd_fdinfo.flags,
> + event_pidfd_fdinfo.mnt_id,
> + event_pidfd_fdinfo.pid,
> + event_pidfd_fdinfo.ns_pid);
> goto next_event;
> } else {
> tst_res(TPASS,
> @@ -342,9 +431,20 @@ next_event:
> if (info && info->pidfd >= 0)
> SAFE_CLOSE(info->pidfd);
> + }
> - if (event_pidfd_fdinfo)
> - free(event_pidfd_fdinfo);
> + if (tc->trigger_in_child && !tc->want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + if (TST_VARIANT_PIDFD_THREAD) {
> + SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> + } else {
> + SAFE_WAITPID(worker_id.pid, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> + }
> }
> }
> @@ -352,19 +452,17 @@ static void do_cleanup(void)
> {
> if (fanotify_fd >= 0)
> SAFE_CLOSE(fanotify_fd);
> -
> - if (self_pidfd_fdinfo)
> - free(self_pidfd_fdinfo);
> }
> static struct tst_test test = {
> .setup = do_setup,
> .test = do_test,
> .tcnt = ARRAY_SIZE(test_cases),
> - .test_variants = 2,
> + .test_variants = 4,
> .cleanup = do_cleanup,
> .all_filesystems = 1,
> .needs_root = 1,
> + .needs_checkpoints = 1,
> .mount_device = 1,
> .mntpoint = MOUNT_PATH,
> .forks_child = 1,
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [LTP] [PATCH] fanotify: prepare tests for thread pidfd reporting
@ 2026-05-27 7:23 ` Petr Vorel
0 siblings, 0 replies; 21+ messages in thread
From: Petr Vorel @ 2026-05-27 7:23 UTC (permalink / raw)
To: AnonymeMeow; +Cc: jack, amir73il, linux-kernel, repnop, linux-fsdevel, ltp
Hi AnonymeMeow, Amir,
[ Cc Cyril ]
> On 2026-05-25 12:04:32+02:00, Amir Goldstein wrote:
> > On Sun, May 24, 2026 at 12:25 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
@AnonymeMeow nit: if you put whole conversation after --- (below), it will not
be part of the commit message when one applies the patch. And here should be a
proper commit message.
FYI We have just 2 days before releasing new LTP version, this will not get in
anyway. At least I'm not sure if we want to risk regression in the release for
functionality which is not yet even in the next tree.
Also I found that fanotify21.c on the current master fails when running more
than 1 iteration:
# ./fanotify21 -i2
...
tst_test.c:1980: TINFO: === Testing on ext2 ===
tst_test.c:1291: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.4 (6-Mar-2025)
tst_test.c:1303: TINFO: Mounting /dev/loop0 to /tmp/LTP_fanWZr7p3/fs_mnt fstyp=ext2 flags=0
fanotify21.c:238: TINFO: Test #0.0: return a valid pidfd for event created by self
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:415: TPASS: got an event with a valid pidfd info record, mask: 32, pid: 234527, fd: 4, pidfd: 5, info_type: 4, info_len: 8
fanotify21.c:238: TINFO: Test #1.0: return a valid pidfd for event created by child
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:415: TPASS: got an event with a valid pidfd info record, mask: 32, pid: 234528, fd: 4, pidfd: 5, info_type: 4, info_len: 8
fanotify21.c:238: TINFO: Test #2.0: return invalid pidfd for event created by terminated child
fanotify21.c:340: TPASS: event->fd 4 is valid as expected
fanotify21.c:375: TPASS: pid: 234529 terminated before pidfd was created, pidfd set to the value of: -1, as expected
fanotify21.c:238: TINFO: Test #3.0: fail to open rw fd for event created on read-only mount
fanotify21.c:297: TPASS: cannot read event with rw fd from a ro fs
fanotify21.c:238: TINFO: Test #0.0: return a valid pidfd for event created by self
fanotify21.c:300: TBROK: reading fanotify events failed: EROFS (30)
Summary:
passed 7
failed 0
broken 1
skipped 0
warnings 0
That should be fixed. It might be worth to fix that before the release.
Kind regards,
Petr
> > > The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> > > exclusive because by the time the pidfd support was introduced to
> > > fanotify, pidfds could only be created for thread group leaders. Now
> > > that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> > > this restriction can be lifted.
> > > This patch allows listeners using FAN_REPORT_PIDFD | FAN_REPORT_TID
> > > to receive the pidfd referring to the thread that triggered the
> > > event.
> > > Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> > Regarding the legal standpoint, as far as I can tell, commit d4563201f33a0
> > ("Documentation: simplify and clarify DCO contribution example language")
> > explicitly allowed pseudonyms, so although unorthodox, this seems to be legit.
> > In any case, the contributing itself seems legit to me, so
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> > But before merging this, AnonymeMeow, since it is your first contribution
> > to the Linux kernel (right?) as well as a "I am not a robot" challenge
> > I would like to first see that the tests have been dealt with.
> > The LTP test fanotify20 explicitly checks that this flag combination is not
> > allowed. Need to discuss an acceptable solution with LTP developers -
> > remove this check depending on kernel version or whatever they choose.
> > The LTP test fanotify21 tests the FAN_REPORT_PIDFD functionality.
> > Please add two new variants for REPORT_TID:
> > .test_variants = 4,
> > #define TST_VARIANT_FD_ERROR (tst_variant & 1)
> > #define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
> > The existing checks if (tst_variant) become if (TST_VARIANT_FD_ERROR)
> > so bit 0 represents the FAN_REPORT_FD_ERROR variation.
> > in do_setup() need to use if (TST_VARIANT_PIDFD_THREAD)
> > to add FAN_REPORT_TID and to use PIDFD_THREAD for pidfd.
> > Follow the footsteps of fd_error_unsupported to skip the
> > TST_VARIANT_PIDFD_THREAD variants on kernels where the flag
> > combination is not supported.
> > Good luck,
> > Amir.
> Thanks for the helpful and detailed guidance on how to extend the tests!
> I've updated the test cases according to your suggestions.
> After applying my previous patch, the LTP fanotify20 test needs to probe
> whether the running kernel supports thread pidfd reporting. I didn't use
> kernel version check because the LTP documentation notes that kernel
> versions do not always correspond to a well-defined feature set due to
> distribution backports, and recommends runtime feature checks. I probe
> whether fanotify_init() succeeds with FAN_REPORT_PIDFD | FAN_REPORT_TID
> as a workaround. The first test case uses exactly the same flags, but it
> still verifies the return value and errno, so I am not sure whether this
> should be considered duplicate logic. I also added one test case that
> passes all relevant FAN_REPORT_* flags and verifies whether
> fanotify_init() behaves as expected. The expected result depends on the
> previous probe result.
> Also, I added one test case and two test variants to LTP test
> fanotify21. The new test case verifies that fanotify reports the correct
> pidfd for the event generated by a child process or a worker thread
> created by the main thread. This is especially useful for the thread
> pidfd test variants, because it checks that the pidfd reported by
> fanotify really refers to the event-generating thread rather than to
> the thread-group leader. The two new test variants extend the existing
> ones by adding the thread pidfd mode, as you suggested.
> I ran all fanotify-related LTP tests on both a vanilla Linux 7.1-rc5
> kernel and a Linux 7.1-rc5 kernel with my patch applied. Both kernels
> passed all the tests. They were both built with the config file from the
> official Arch Linux package repo. More specifically, both kernels passed
> all three test cases in fanotify20, with no skips. The test cases expect
> different results from the two kernels, which is the expected behavior.
> For fanotify21, the patched kernel passed all test cases in all test
> variants. On the unpatched kernel, all test cases passed for variants
> without TST_VARIANT_PIDFD_THREAD, while all test cases in variants with
> TST_VARIANT_PIDFD_THREAD were skipped, which is also the expected
> behavior.
> Again, thanks for the detailed guidance on this!
> With Best Regards,
> AnonymeMeow
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
> testcases/kernel/syscalls/fanotify/Makefile | 2 +-
> .../kernel/syscalls/fanotify/fanotify20.c | 24 +-
> .../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
> 3 files changed, 184 insertions(+), 66 deletions(-)
> diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
> index 3628094ba..b20bb50e9 100644
> --- a/testcases/kernel/syscalls/fanotify/Makefile
> +++ b/testcases/kernel/syscalls/fanotify/Makefile
> @@ -2,7 +2,7 @@
> # Copyright (c) Jan Kara <jack@suse.cz>, 2013
> top_srcdir ?= ../../../..
> -fanotify11: CFLAGS+=-pthread
> +fanotify11 fanotify21: CFLAGS+=-pthread
> include $(top_srcdir)/include/mk/testcases.mk
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index b32ecf6aa..5d77b485c 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -28,19 +28,27 @@
> #define FLAGS_DESC(x) .flags = x, .desc = #x
> static int fd;
> +static int thread_pidfd_unsupported;
> static struct test_case_t {
> unsigned int flags;
> char *desc;
> int exp_errno;
> + unsigned int needs_thread_pidfd;
> } test_cases[] = {
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
> .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> },
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> },
> + {
> + FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> + .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> + },
> };
> static void do_setup(void)
> @@ -51,17 +59,29 @@ static void do_setup(void)
> */
> REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
> MOUNT_PATH);
> +
> + /*
> + * Check whether the kernel supports FAN_REPORT_PIDFD in combination
> + * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
> + * set expect different errno values depending on whether this
> + * combination is supported.
> + */
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
> }
> static void do_test(unsigned int i)
> {
> struct test_case_t *tc = &test_cases[i];
> - tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
> + int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
> + 0 : tc->exp_errno;
> +
> + tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
> tc->desc);
> TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
> - tc->exp_errno);
> + exp_errno);
> if (fd > 0)
> SAFE_CLOSE(fd);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> index 340fb0018..4629860da 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify21.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -20,9 +20,11 @@
> #include <stdlib.h>
> #include <string.h>
> #include <sys/mount.h>
> +#include <pthread.h>
> #include "tst_test.h"
> #include "tst_safe_stdio.h"
> #include "tst_safe_macros.h"
> +#include "tst_safe_pthread.h"
> #include "lapi/pidfd.h"
> #ifdef HAVE_SYS_FANOTIFY_H
> @@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
> static struct test_case_t {
> char *name;
> - int fork;
> + int trigger_in_child;
> int want_pidfd_err;
> int remount_ro;
> } test_cases[] = {
> @@ -52,6 +54,12 @@ static struct test_case_t {
> 0,
> 0,
> },
> + {
> + "return a valid pidfd for event created by child",
> + 1,
> + 0,
> + 0,
> + },
> {
> "return invalid pidfd for event created by terminated child",
> 1,
> @@ -68,16 +76,17 @@ static struct test_case_t {
> static int fanotify_fd;
> static char event_buf[BUF_SZ];
> -static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
> +static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
> static int fd_error_unsupported;
> +static int thread_pidfd_unsupported;
> +
> +#define TST_VARIANT_FD_ERROR (tst_variant & 1)
> +#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
> -static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> +static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
> {
> char *fdinfo_path;
> - struct pidfd_fdinfo_t *pidfd_fdinfo;
> -
> - pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
> SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
> SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
> @@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
> free(fdinfo_path);
> -
> - return pidfd_fdinfo;
> }
> static void generate_event(void)
> @@ -100,30 +107,91 @@ static void generate_event(void)
> SAFE_CLOSE(fd);
> }
> -static void do_fork(void)
> +static pid_t do_fork(int want_pidfd_err)
> {
> - int status;
> + int pidfd;
> pid_t child;
> child = SAFE_FORK();
> if (child == 0) {
> SAFE_CLOSE(fanotify_fd);
> generate_event();
> + TST_CHECKPOINT_WAIT(0);
> exit(EXIT_SUCCESS);
> }
> - SAFE_WAITPID(child, &status, 0);
> - if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> - tst_brk(TBROK,
> - "child process terminated incorrectly");
> + pidfd = SAFE_PIDFD_OPEN(child, 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> + if (want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_WAITPID(child, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> +
> + return -1;
> + }
> +
> + return child;
> }
> -static void do_setup(void)
> +static void *thread_generate_event(void *arg)
> +{
> + *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
> + TST_CHECKPOINT_WAKE(0);
> +
> + generate_event();
> + TST_CHECKPOINT_WAIT(0);
> + pthread_exit(0);
> +}
> +
> +static pthread_t do_pthread_create(int want_pidfd_err)
> {
> int pidfd;
> + pthread_t worker;
> +
> + SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
> +
> + TST_CHECKPOINT_WAIT(0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> +
> + if (want_pidfd_err) {
> + int status;
> + struct pidfd_fdinfo_t thread_pidfd_fdinfo;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_PTHREAD_JOIN(worker, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> +
> + /*
> + * Unlike waitpid(), pthread_join() only waits until the worker thread
> + * has exited from the pthread point of view. The thread may still be
> + * visible through its pidfd for a short time afterwards, and fanotify
> + * creates the event pidfd when the event is read. Wait until the
> + * worker pidfd fdinfo reports Pid: -1 before reading the event so
> + * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
> + */
> + do {
> + read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
> + } while (thread_pidfd_fdinfo.pid != -1);
> +
> + SAFE_CLOSE(pidfd);
> +
> + return -1;
> + }
> +
> + SAFE_CLOSE(pidfd);
> +
> + return worker;
> +}
> +
> +static void do_setup(void)
> +{
> int init_flags = FAN_REPORT_PIDFD;
> - if (tst_variant) {
> + if (TST_VARIANT_FD_ERROR) {
> fanotify_fd = -1;
> fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
> if (fd_error_unsupported)
> @@ -131,6 +199,15 @@ static void do_setup(void)
> init_flags |= FAN_REPORT_FD_ERROR;
> }
> + if (TST_VARIANT_PIDFD_THREAD) {
> + fanotify_fd = -1;
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
> + if (thread_pidfd_unsupported)
> + return;
> + init_flags |= FAN_REPORT_TID;
> + }
> +
> SAFE_TOUCH(TEST_FILE, 0666, NULL);
> /*
> @@ -144,15 +221,6 @@ static void do_setup(void)
> fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
> SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
> TEST_FILE);
> -
> - pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
> -
> - self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
> - if (self_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "pidfd=%d, failed to read pidfd fdinfo",
> - pidfd);
> - }
> }
> static void do_test(unsigned int num)
> @@ -160,17 +228,29 @@ static void do_test(unsigned int num)
> int i = 0, len;
> struct test_case_t *tc = &test_cases[num];
> int nopidfd_err = tc->want_pidfd_err ?
> - (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
> - int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
> + (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
> + int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
> + union {
> + pid_t pid;
> + pthread_t pthread_id;
> + } worker_id;
> tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
> - tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
> + TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
> + "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
> + (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
> - if (fd_error_unsupported && tst_variant) {
> + if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
> FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
> return;
> }
> + if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
> + FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
> + thread_pidfd_unsupported);
> + return;
> + }
> +
> if (tc->remount_ro) {
> /* SAFE_MOUNT fails to remount FUSE */
> if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> @@ -182,14 +262,30 @@ static void do_test(unsigned int num)
> }
> /*
> - * Generate the event in either self or a child process. Event
> - * generation in a child process is done so that the FAN_NOPIDFD case
> - * can be verified.
> + * Generate the event either in the current task or in another task.
> + * When trigger_in_child is set, the event can be generated by either
> + * a child process or a worker thread depending on the test variant.
> + * The want_pidfd_err field determines whether the event-generating
> + * task is still valid when the event is read.
> */
> - if (tc->fork)
> - do_fork();
> - else
> + if (tc->trigger_in_child) {
> + if (TST_VARIANT_PIDFD_THREAD)
> + worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
> + else
> + worker_id.pid = do_fork(tc->want_pidfd_err);
> + } else {
> + /*
> + * Although the expected pid and the pid reported by fanotify are
> + * the same in this case, pidfds created with and without PIDFD_THREAD
> + * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
> + * pidfd fdinfo so that the fdinfo can be compared bitwise.
> + */
> + int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> generate_event();
> + }
> /*
> * Read all of the queued events into the provided event
> @@ -208,7 +304,7 @@ static void do_test(unsigned int num)
> while (i < len) {
> struct fanotify_event_metadata *event;
> struct fanotify_event_info_pidfd *info;
> - struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
> + struct pidfd_fdinfo_t event_pidfd_fdinfo;
> event = (struct fanotify_event_metadata *)&event_buf[i];
> info = (struct fanotify_event_info_pidfd *)(event + 1);
> @@ -288,39 +384,32 @@ static void do_test(unsigned int num)
> * No pidfd errors occurred, continue with verifying pidfd
> * fdinfo validity.
> */
> - event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
> - if (event_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "reading fdinfo for pidfd: %d "
> - "describing pid: %u failed",
> - info->pidfd,
> - (unsigned int)event->pid);
> - goto next_event;
> - } else if (event_pidfd_fdinfo->pid != event->pid) {
> + read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
> + if (event_pidfd_fdinfo.pid != event->pid) {
> tst_res(TFAIL,
> "pidfd provided for incorrect pid "
> "(expected pidfd for pid: %u, got pidfd for "
> "pid: %u)",
> (unsigned int)event->pid,
> - (unsigned int)event_pidfd_fdinfo->pid);
> + (unsigned int)event_pidfd_fdinfo.pid);
> goto next_event;
> - } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
> + } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
> sizeof(struct pidfd_fdinfo_t))) {
> tst_res(TFAIL,
> "pidfd fdinfo values for self and event differ "
> "(expected pos: %d, flags: %x, mnt_id: %d, "
> "pid: %d, ns_pid: %d, got pos: %d, "
> "flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
> - self_pidfd_fdinfo->pos,
> - self_pidfd_fdinfo->flags,
> - self_pidfd_fdinfo->mnt_id,
> - self_pidfd_fdinfo->pid,
> - self_pidfd_fdinfo->ns_pid,
> - event_pidfd_fdinfo->pos,
> - event_pidfd_fdinfo->flags,
> - event_pidfd_fdinfo->mnt_id,
> - event_pidfd_fdinfo->pid,
> - event_pidfd_fdinfo->ns_pid);
> + expected_pidfd_fdinfo.pos,
> + expected_pidfd_fdinfo.flags,
> + expected_pidfd_fdinfo.mnt_id,
> + expected_pidfd_fdinfo.pid,
> + expected_pidfd_fdinfo.ns_pid,
> + event_pidfd_fdinfo.pos,
> + event_pidfd_fdinfo.flags,
> + event_pidfd_fdinfo.mnt_id,
> + event_pidfd_fdinfo.pid,
> + event_pidfd_fdinfo.ns_pid);
> goto next_event;
> } else {
> tst_res(TPASS,
> @@ -342,9 +431,20 @@ next_event:
> if (info && info->pidfd >= 0)
> SAFE_CLOSE(info->pidfd);
> + }
> - if (event_pidfd_fdinfo)
> - free(event_pidfd_fdinfo);
> + if (tc->trigger_in_child && !tc->want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + if (TST_VARIANT_PIDFD_THREAD) {
> + SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> + } else {
> + SAFE_WAITPID(worker_id.pid, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> + }
> }
> }
> @@ -352,19 +452,17 @@ static void do_cleanup(void)
> {
> if (fanotify_fd >= 0)
> SAFE_CLOSE(fanotify_fd);
> -
> - if (self_pidfd_fdinfo)
> - free(self_pidfd_fdinfo);
> }
> static struct tst_test test = {
> .setup = do_setup,
> .test = do_test,
> .tcnt = ARRAY_SIZE(test_cases),
> - .test_variants = 2,
> + .test_variants = 4,
> .cleanup = do_cleanup,
> .all_filesystems = 1,
> .needs_root = 1,
> + .needs_checkpoints = 1,
> .mount_device = 1,
> .mntpoint = MOUNT_PATH,
> .forks_child = 1,
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread* [PATCH v2 1/2] fanotify: fix crash when running multiple iterations
2026-05-27 7:23 ` [LTP] " Petr Vorel
@ 2026-05-27 19:50 ` AnonymeMeow
-1 siblings, 0 replies; 21+ messages in thread
From: AnonymeMeow @ 2026-05-27 19:50 UTC (permalink / raw)
To: pvorel
Cc: amir73il, jack, repnop, linux-kernel, linux-fsdevel, ltp,
AnonymeMeow
This commit fixes ./fanotify13 -i10 crash by restoring the deleted objects
every time a test finishes.
This commit also fixes ./fanotify21 -i10 crash by remounting the read-only
mount (if the mount is remounted read-only during the test) back to
read-write every time a test finishes.
Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
On 2026-05-27 09:23:12+02:00, Petr Vorel wrote:
> @AnonymeMeow nit: if you put whole conversation after --- (below), it will not
> be part of the commit message when one applies the patch. And here should be a
> proper commit message.
Thank you for pointing this out, I will be more mindful the next time
(this time).
> Also I found that fanotify21.c on the current master fails when running more
> than 1 iteration:
Thanks for testing my patch, I didn't know that the standalone test binary
supports the -i option to specify the number of iterations. Running
multiple iterations through kirk's -i option seems to work fine.
I then tested all fanotify tests with the -i option and found that
fanotify13 also crashed when running multiple iterations. I fixed both
fanotify13 and fanotify21 in this patch. So now all fanotify tests pass
with the -i option.
With Best Regards,
AnonymeMeow
---
.../kernel/syscalls/fanotify/fanotify13.c | 20 +++++++++++++++++++
.../kernel/syscalls/fanotify/fanotify21.c | 13 +++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 76d40eaf7..540e0b483 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -137,6 +137,16 @@ static void delete_objects(void)
}
}
+static void clean_upper_dir(void)
+{
+ unsigned int i;
+
+ SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
+ for (i = 0; i < ARRAY_SIZE(objects); i++)
+ SAFE_UNLINK(objects[i].path);
+ SAFE_UMOUNT(MOUNT_PATH);
+}
+
static void get_object_stats(void)
{
unsigned int i;
@@ -340,6 +350,15 @@ static void do_test(unsigned int number)
"Did not get an expected event (expected: %llx)",
event_set[i].expected_mask);
}
+
+ if (tc->mask & FAN_DELETE_SELF) {
+ if (tst_variant & 1) {
+ clean_upper_dir();
+ } else {
+ create_objects();
+ get_object_stats();
+ }
+ }
out:
SAFE_CLOSE(fanotify_fd);
}
@@ -417,6 +436,7 @@ static void do_cleanup(void)
if (ovl_bind_mounted)
SAFE_UMOUNT(MOUNT_PATH);
if (bind_mounted) {
+ delete_objects();
SAFE_UMOUNT(MOUNT_PATH);
SAFE_RMDIR(MOUNT_PATH);
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 340fb0018..2e3dbd4bd 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -199,7 +199,7 @@ static void do_test(unsigned int num)
if (len < 0) {
if (tc->remount_ro && !fd_err && errno == EROFS) {
tst_res(TPASS, "cannot read event with rw fd from a ro fs");
- return;
+ goto restore_rw_mount;
}
tst_brk(TBROK | TERRNO, "reading fanotify events failed");
} else if (tc->remount_ro && !fd_err) {
@@ -346,6 +346,17 @@ next_event:
if (event_pidfd_fdinfo)
free(event_pidfd_fdinfo);
}
+
+restore_rw_mount:
+ if (tc->remount_ro) {
+ /* SAFE_MOUNT fails to remount FUSE */
+ if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
+ MS_REMOUNT, NULL) != 0) {
+ tst_brk(TFAIL,
+ "filesystem %s failed to remount read-write",
+ tst_device->fs_type);
+ }
+ }
}
static void do_cleanup(void)
--
2.54.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [LTP] [PATCH v2 1/2] fanotify: fix crash when running multiple iterations
@ 2026-05-27 19:50 ` AnonymeMeow
0 siblings, 0 replies; 21+ messages in thread
From: AnonymeMeow @ 2026-05-27 19:50 UTC (permalink / raw)
To: pvorel
Cc: jack, amir73il, linux-kernel, AnonymeMeow, repnop, linux-fsdevel,
ltp
This commit fixes ./fanotify13 -i10 crash by restoring the deleted objects
every time a test finishes.
This commit also fixes ./fanotify21 -i10 crash by remounting the read-only
mount (if the mount is remounted read-only during the test) back to
read-write every time a test finishes.
Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
On 2026-05-27 09:23:12+02:00, Petr Vorel wrote:
> @AnonymeMeow nit: if you put whole conversation after --- (below), it will not
> be part of the commit message when one applies the patch. And here should be a
> proper commit message.
Thank you for pointing this out, I will be more mindful the next time
(this time).
> Also I found that fanotify21.c on the current master fails when running more
> than 1 iteration:
Thanks for testing my patch, I didn't know that the standalone test binary
supports the -i option to specify the number of iterations. Running
multiple iterations through kirk's -i option seems to work fine.
I then tested all fanotify tests with the -i option and found that
fanotify13 also crashed when running multiple iterations. I fixed both
fanotify13 and fanotify21 in this patch. So now all fanotify tests pass
with the -i option.
With Best Regards,
AnonymeMeow
---
.../kernel/syscalls/fanotify/fanotify13.c | 20 +++++++++++++++++++
.../kernel/syscalls/fanotify/fanotify21.c | 13 +++++++++++-
2 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 76d40eaf7..540e0b483 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -137,6 +137,16 @@ static void delete_objects(void)
}
}
+static void clean_upper_dir(void)
+{
+ unsigned int i;
+
+ SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
+ for (i = 0; i < ARRAY_SIZE(objects); i++)
+ SAFE_UNLINK(objects[i].path);
+ SAFE_UMOUNT(MOUNT_PATH);
+}
+
static void get_object_stats(void)
{
unsigned int i;
@@ -340,6 +350,15 @@ static void do_test(unsigned int number)
"Did not get an expected event (expected: %llx)",
event_set[i].expected_mask);
}
+
+ if (tc->mask & FAN_DELETE_SELF) {
+ if (tst_variant & 1) {
+ clean_upper_dir();
+ } else {
+ create_objects();
+ get_object_stats();
+ }
+ }
out:
SAFE_CLOSE(fanotify_fd);
}
@@ -417,6 +436,7 @@ static void do_cleanup(void)
if (ovl_bind_mounted)
SAFE_UMOUNT(MOUNT_PATH);
if (bind_mounted) {
+ delete_objects();
SAFE_UMOUNT(MOUNT_PATH);
SAFE_RMDIR(MOUNT_PATH);
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 340fb0018..2e3dbd4bd 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -199,7 +199,7 @@ static void do_test(unsigned int num)
if (len < 0) {
if (tc->remount_ro && !fd_err && errno == EROFS) {
tst_res(TPASS, "cannot read event with rw fd from a ro fs");
- return;
+ goto restore_rw_mount;
}
tst_brk(TBROK | TERRNO, "reading fanotify events failed");
} else if (tc->remount_ro && !fd_err) {
@@ -346,6 +346,17 @@ next_event:
if (event_pidfd_fdinfo)
free(event_pidfd_fdinfo);
}
+
+restore_rw_mount:
+ if (tc->remount_ro) {
+ /* SAFE_MOUNT fails to remount FUSE */
+ if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
+ MS_REMOUNT, NULL) != 0) {
+ tst_brk(TFAIL,
+ "filesystem %s failed to remount read-write",
+ tst_device->fs_type);
+ }
+ }
}
static void do_cleanup(void)
--
2.54.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v2 2/2] fanotify: prepare tests for thread pidfd reporting
2026-05-27 19:50 ` [LTP] " AnonymeMeow
@ 2026-05-27 19:50 ` AnonymeMeow
-1 siblings, 0 replies; 21+ messages in thread
From: AnonymeMeow @ 2026-05-27 19:50 UTC (permalink / raw)
To: pvorel
Cc: amir73il, jack, repnop, linux-kernel, linux-fsdevel, ltp,
AnonymeMeow
Add a runtime probe in fanotify20 for FAN_REPORT_PIDFD combined with
FAN_REPORT_TID, and use the probe result to decide the expected
fanotify_init() behavior. This keeps the test compatible with both kernels
that do and do not support thread pidfd reporting. Also add a test case
that combines all FAN_REPORT_* flags supported since Linux 5.15.
Add two test variants to fanotify21 that run the existing pidfd tests in
thread-pidfd mode. And add a test case that verifies pidfd reporting for
events generated by another task, either a child process or a worker
thread depending on the test variant. This test case especially ensures
that the pidfd reported by fanotify refers to the event-generating thread
when thread pidfds are enabled, rather than referring to the thread-group
leader.
Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
This patch is almost idential to the v1 patch, except that it is now based
on patch 1/2.
Link: https://lore.kernel.org/lkml/20260527064041.50443-1-anonymemeow@gmail.com/
---
testcases/kernel/syscalls/fanotify/Makefile | 2 +-
.../kernel/syscalls/fanotify/fanotify20.c | 24 +-
.../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
3 files changed, 184 insertions(+), 66 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
index 3628094ba..b20bb50e9 100644
--- a/testcases/kernel/syscalls/fanotify/Makefile
+++ b/testcases/kernel/syscalls/fanotify/Makefile
@@ -2,7 +2,7 @@
# Copyright (c) Jan Kara <jack@suse.cz>, 2013
top_srcdir ?= ../../../..
-fanotify11: CFLAGS+=-pthread
+fanotify11 fanotify21: CFLAGS+=-pthread
include $(top_srcdir)/include/mk/testcases.mk
include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index b32ecf6aa..5d77b485c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -28,19 +28,27 @@
#define FLAGS_DESC(x) .flags = x, .desc = #x
static int fd;
+static int thread_pidfd_unsupported;
static struct test_case_t {
unsigned int flags;
char *desc;
int exp_errno;
+ unsigned int needs_thread_pidfd;
} test_cases[] = {
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
.exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
},
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
},
+ {
+ FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
+ .exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
+ },
};
static void do_setup(void)
@@ -51,17 +59,29 @@ static void do_setup(void)
*/
REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
MOUNT_PATH);
+
+ /*
+ * Check whether the kernel supports FAN_REPORT_PIDFD in combination
+ * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
+ * set expect different errno values depending on whether this
+ * combination is supported.
+ */
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
}
static void do_test(unsigned int i)
{
struct test_case_t *tc = &test_cases[i];
- tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
+ int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
+ 0 : tc->exp_errno;
+
+ tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
tc->desc);
TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
- tc->exp_errno);
+ exp_errno);
if (fd > 0)
SAFE_CLOSE(fd);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 2e3dbd4bd..0c190ab6f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -20,9 +20,11 @@
#include <stdlib.h>
#include <string.h>
#include <sys/mount.h>
+#include <pthread.h>
#include "tst_test.h"
#include "tst_safe_stdio.h"
#include "tst_safe_macros.h"
+#include "tst_safe_pthread.h"
#include "lapi/pidfd.h"
#ifdef HAVE_SYS_FANOTIFY_H
@@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
static struct test_case_t {
char *name;
- int fork;
+ int trigger_in_child;
int want_pidfd_err;
int remount_ro;
} test_cases[] = {
@@ -52,6 +54,12 @@ static struct test_case_t {
0,
0,
},
+ {
+ "return a valid pidfd for event created by child",
+ 1,
+ 0,
+ 0,
+ },
{
"return invalid pidfd for event created by terminated child",
1,
@@ -68,16 +76,17 @@ static struct test_case_t {
static int fanotify_fd;
static char event_buf[BUF_SZ];
-static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
+static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
static int fd_error_unsupported;
+static int thread_pidfd_unsupported;
+
+#define TST_VARIANT_FD_ERROR (tst_variant & 1)
+#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
-static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
+static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
{
char *fdinfo_path;
- struct pidfd_fdinfo_t *pidfd_fdinfo;
-
- pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
@@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
free(fdinfo_path);
-
- return pidfd_fdinfo;
}
static void generate_event(void)
@@ -100,30 +107,91 @@ static void generate_event(void)
SAFE_CLOSE(fd);
}
-static void do_fork(void)
+static pid_t do_fork(int want_pidfd_err)
{
- int status;
+ int pidfd;
pid_t child;
child = SAFE_FORK();
if (child == 0) {
SAFE_CLOSE(fanotify_fd);
generate_event();
+ TST_CHECKPOINT_WAIT(0);
exit(EXIT_SUCCESS);
}
- SAFE_WAITPID(child, &status, 0);
- if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
- tst_brk(TBROK,
- "child process terminated incorrectly");
+ pidfd = SAFE_PIDFD_OPEN(child, 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
+ if (want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_WAITPID(child, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+
+ return -1;
+ }
+
+ return child;
}
-static void do_setup(void)
+static void *thread_generate_event(void *arg)
+{
+ *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
+ TST_CHECKPOINT_WAKE(0);
+
+ generate_event();
+ TST_CHECKPOINT_WAIT(0);
+ pthread_exit(0);
+}
+
+static pthread_t do_pthread_create(int want_pidfd_err)
{
int pidfd;
+ pthread_t worker;
+
+ SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
+
+ TST_CHECKPOINT_WAIT(0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+
+ if (want_pidfd_err) {
+ int status;
+ struct pidfd_fdinfo_t thread_pidfd_fdinfo;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_PTHREAD_JOIN(worker, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+
+ /*
+ * Unlike waitpid(), pthread_join() only waits until the worker thread
+ * has exited from the pthread point of view. The thread may still be
+ * visible through its pidfd for a short time afterwards, and fanotify
+ * creates the event pidfd when the event is read. Wait until the
+ * worker pidfd fdinfo reports Pid: -1 before reading the event so
+ * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
+ */
+ do {
+ read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
+ } while (thread_pidfd_fdinfo.pid != -1);
+
+ SAFE_CLOSE(pidfd);
+
+ return -1;
+ }
+
+ SAFE_CLOSE(pidfd);
+
+ return worker;
+}
+
+static void do_setup(void)
+{
int init_flags = FAN_REPORT_PIDFD;
- if (tst_variant) {
+ if (TST_VARIANT_FD_ERROR) {
fanotify_fd = -1;
fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
if (fd_error_unsupported)
@@ -131,6 +199,15 @@ static void do_setup(void)
init_flags |= FAN_REPORT_FD_ERROR;
}
+ if (TST_VARIANT_PIDFD_THREAD) {
+ fanotify_fd = -1;
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
+ if (thread_pidfd_unsupported)
+ return;
+ init_flags |= FAN_REPORT_TID;
+ }
+
SAFE_TOUCH(TEST_FILE, 0666, NULL);
/*
@@ -144,15 +221,6 @@ static void do_setup(void)
fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
TEST_FILE);
-
- pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
-
- self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
- if (self_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "pidfd=%d, failed to read pidfd fdinfo",
- pidfd);
- }
}
static void do_test(unsigned int num)
@@ -160,17 +228,29 @@ static void do_test(unsigned int num)
int i = 0, len;
struct test_case_t *tc = &test_cases[num];
int nopidfd_err = tc->want_pidfd_err ?
- (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
- int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
+ (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
+ int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
+ union {
+ pid_t pid;
+ pthread_t pthread_id;
+ } worker_id;
tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
- tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
+ TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
+ "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
+ (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
- if (fd_error_unsupported && tst_variant) {
+ if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
return;
}
+ if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
+ FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
+ thread_pidfd_unsupported);
+ return;
+ }
+
if (tc->remount_ro) {
/* SAFE_MOUNT fails to remount FUSE */
if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
@@ -182,14 +262,30 @@ static void do_test(unsigned int num)
}
/*
- * Generate the event in either self or a child process. Event
- * generation in a child process is done so that the FAN_NOPIDFD case
- * can be verified.
+ * Generate the event either in the current task or in another task.
+ * When trigger_in_child is set, the event can be generated by either
+ * a child process or a worker thread depending on the test variant.
+ * The want_pidfd_err field determines whether the event-generating
+ * task is still valid when the event is read.
*/
- if (tc->fork)
- do_fork();
- else
+ if (tc->trigger_in_child) {
+ if (TST_VARIANT_PIDFD_THREAD)
+ worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
+ else
+ worker_id.pid = do_fork(tc->want_pidfd_err);
+ } else {
+ /*
+ * Although the expected pid and the pid reported by fanotify are
+ * the same in this case, pidfds created with and without PIDFD_THREAD
+ * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
+ * pidfd fdinfo so that the fdinfo can be compared bitwise.
+ */
+ int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
generate_event();
+ }
/*
* Read all of the queued events into the provided event
@@ -208,7 +304,7 @@ static void do_test(unsigned int num)
while (i < len) {
struct fanotify_event_metadata *event;
struct fanotify_event_info_pidfd *info;
- struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
+ struct pidfd_fdinfo_t event_pidfd_fdinfo;
event = (struct fanotify_event_metadata *)&event_buf[i];
info = (struct fanotify_event_info_pidfd *)(event + 1);
@@ -288,39 +384,32 @@ static void do_test(unsigned int num)
* No pidfd errors occurred, continue with verifying pidfd
* fdinfo validity.
*/
- event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
- if (event_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "reading fdinfo for pidfd: %d "
- "describing pid: %u failed",
- info->pidfd,
- (unsigned int)event->pid);
- goto next_event;
- } else if (event_pidfd_fdinfo->pid != event->pid) {
+ read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
+ if (event_pidfd_fdinfo.pid != event->pid) {
tst_res(TFAIL,
"pidfd provided for incorrect pid "
"(expected pidfd for pid: %u, got pidfd for "
"pid: %u)",
(unsigned int)event->pid,
- (unsigned int)event_pidfd_fdinfo->pid);
+ (unsigned int)event_pidfd_fdinfo.pid);
goto next_event;
- } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
+ } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
sizeof(struct pidfd_fdinfo_t))) {
tst_res(TFAIL,
"pidfd fdinfo values for self and event differ "
"(expected pos: %d, flags: %x, mnt_id: %d, "
"pid: %d, ns_pid: %d, got pos: %d, "
"flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
- self_pidfd_fdinfo->pos,
- self_pidfd_fdinfo->flags,
- self_pidfd_fdinfo->mnt_id,
- self_pidfd_fdinfo->pid,
- self_pidfd_fdinfo->ns_pid,
- event_pidfd_fdinfo->pos,
- event_pidfd_fdinfo->flags,
- event_pidfd_fdinfo->mnt_id,
- event_pidfd_fdinfo->pid,
- event_pidfd_fdinfo->ns_pid);
+ expected_pidfd_fdinfo.pos,
+ expected_pidfd_fdinfo.flags,
+ expected_pidfd_fdinfo.mnt_id,
+ expected_pidfd_fdinfo.pid,
+ expected_pidfd_fdinfo.ns_pid,
+ event_pidfd_fdinfo.pos,
+ event_pidfd_fdinfo.flags,
+ event_pidfd_fdinfo.mnt_id,
+ event_pidfd_fdinfo.pid,
+ event_pidfd_fdinfo.ns_pid);
goto next_event;
} else {
tst_res(TPASS,
@@ -342,9 +431,20 @@ next_event:
if (info && info->pidfd >= 0)
SAFE_CLOSE(info->pidfd);
+ }
- if (event_pidfd_fdinfo)
- free(event_pidfd_fdinfo);
+ if (tc->trigger_in_child && !tc->want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ if (TST_VARIANT_PIDFD_THREAD) {
+ SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+ } else {
+ SAFE_WAITPID(worker_id.pid, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+ }
}
restore_rw_mount:
@@ -363,19 +463,17 @@ static void do_cleanup(void)
{
if (fanotify_fd >= 0)
SAFE_CLOSE(fanotify_fd);
-
- if (self_pidfd_fdinfo)
- free(self_pidfd_fdinfo);
}
static struct tst_test test = {
.setup = do_setup,
.test = do_test,
.tcnt = ARRAY_SIZE(test_cases),
- .test_variants = 2,
+ .test_variants = 4,
.cleanup = do_cleanup,
.all_filesystems = 1,
.needs_root = 1,
+ .needs_checkpoints = 1,
.mount_device = 1,
.mntpoint = MOUNT_PATH,
.forks_child = 1,
--
2.54.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [LTP] [PATCH v2 2/2] fanotify: prepare tests for thread pidfd reporting
@ 2026-05-27 19:50 ` AnonymeMeow
0 siblings, 0 replies; 21+ messages in thread
From: AnonymeMeow @ 2026-05-27 19:50 UTC (permalink / raw)
To: pvorel
Cc: jack, amir73il, linux-kernel, AnonymeMeow, repnop, linux-fsdevel,
ltp
Add a runtime probe in fanotify20 for FAN_REPORT_PIDFD combined with
FAN_REPORT_TID, and use the probe result to decide the expected
fanotify_init() behavior. This keeps the test compatible with both kernels
that do and do not support thread pidfd reporting. Also add a test case
that combines all FAN_REPORT_* flags supported since Linux 5.15.
Add two test variants to fanotify21 that run the existing pidfd tests in
thread-pidfd mode. And add a test case that verifies pidfd reporting for
events generated by another task, either a child process or a worker
thread depending on the test variant. This test case especially ensures
that the pidfd reported by fanotify refers to the event-generating thread
when thread pidfds are enabled, rather than referring to the thread-group
leader.
Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---
This patch is almost idential to the v1 patch, except that it is now based
on patch 1/2.
Link: https://lore.kernel.org/lkml/20260527064041.50443-1-anonymemeow@gmail.com/
---
testcases/kernel/syscalls/fanotify/Makefile | 2 +-
.../kernel/syscalls/fanotify/fanotify20.c | 24 +-
.../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
3 files changed, 184 insertions(+), 66 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
index 3628094ba..b20bb50e9 100644
--- a/testcases/kernel/syscalls/fanotify/Makefile
+++ b/testcases/kernel/syscalls/fanotify/Makefile
@@ -2,7 +2,7 @@
# Copyright (c) Jan Kara <jack@suse.cz>, 2013
top_srcdir ?= ../../../..
-fanotify11: CFLAGS+=-pthread
+fanotify11 fanotify21: CFLAGS+=-pthread
include $(top_srcdir)/include/mk/testcases.mk
include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index b32ecf6aa..5d77b485c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -28,19 +28,27 @@
#define FLAGS_DESC(x) .flags = x, .desc = #x
static int fd;
+static int thread_pidfd_unsupported;
static struct test_case_t {
unsigned int flags;
char *desc;
int exp_errno;
+ unsigned int needs_thread_pidfd;
} test_cases[] = {
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
.exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
},
{
FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
},
+ {
+ FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
+ .exp_errno = EINVAL,
+ .needs_thread_pidfd = 1,
+ },
};
static void do_setup(void)
@@ -51,17 +59,29 @@ static void do_setup(void)
*/
REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
MOUNT_PATH);
+
+ /*
+ * Check whether the kernel supports FAN_REPORT_PIDFD in combination
+ * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
+ * set expect different errno values depending on whether this
+ * combination is supported.
+ */
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
}
static void do_test(unsigned int i)
{
struct test_case_t *tc = &test_cases[i];
- tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
+ int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
+ 0 : tc->exp_errno;
+
+ tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
tc->desc);
TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
- tc->exp_errno);
+ exp_errno);
if (fd > 0)
SAFE_CLOSE(fd);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 2e3dbd4bd..0c190ab6f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -20,9 +20,11 @@
#include <stdlib.h>
#include <string.h>
#include <sys/mount.h>
+#include <pthread.h>
#include "tst_test.h"
#include "tst_safe_stdio.h"
#include "tst_safe_macros.h"
+#include "tst_safe_pthread.h"
#include "lapi/pidfd.h"
#ifdef HAVE_SYS_FANOTIFY_H
@@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
static struct test_case_t {
char *name;
- int fork;
+ int trigger_in_child;
int want_pidfd_err;
int remount_ro;
} test_cases[] = {
@@ -52,6 +54,12 @@ static struct test_case_t {
0,
0,
},
+ {
+ "return a valid pidfd for event created by child",
+ 1,
+ 0,
+ 0,
+ },
{
"return invalid pidfd for event created by terminated child",
1,
@@ -68,16 +76,17 @@ static struct test_case_t {
static int fanotify_fd;
static char event_buf[BUF_SZ];
-static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
+static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
static int fd_error_unsupported;
+static int thread_pidfd_unsupported;
+
+#define TST_VARIANT_FD_ERROR (tst_variant & 1)
+#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
-static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
+static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
{
char *fdinfo_path;
- struct pidfd_fdinfo_t *pidfd_fdinfo;
-
- pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
@@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
free(fdinfo_path);
-
- return pidfd_fdinfo;
}
static void generate_event(void)
@@ -100,30 +107,91 @@ static void generate_event(void)
SAFE_CLOSE(fd);
}
-static void do_fork(void)
+static pid_t do_fork(int want_pidfd_err)
{
- int status;
+ int pidfd;
pid_t child;
child = SAFE_FORK();
if (child == 0) {
SAFE_CLOSE(fanotify_fd);
generate_event();
+ TST_CHECKPOINT_WAIT(0);
exit(EXIT_SUCCESS);
}
- SAFE_WAITPID(child, &status, 0);
- if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
- tst_brk(TBROK,
- "child process terminated incorrectly");
+ pidfd = SAFE_PIDFD_OPEN(child, 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
+ if (want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_WAITPID(child, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+
+ return -1;
+ }
+
+ return child;
}
-static void do_setup(void)
+static void *thread_generate_event(void *arg)
+{
+ *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
+ TST_CHECKPOINT_WAKE(0);
+
+ generate_event();
+ TST_CHECKPOINT_WAIT(0);
+ pthread_exit(0);
+}
+
+static pthread_t do_pthread_create(int want_pidfd_err)
{
int pidfd;
+ pthread_t worker;
+
+ SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
+
+ TST_CHECKPOINT_WAIT(0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+
+ if (want_pidfd_err) {
+ int status;
+ struct pidfd_fdinfo_t thread_pidfd_fdinfo;
+ TST_CHECKPOINT_WAKE(0);
+ SAFE_PTHREAD_JOIN(worker, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+
+ /*
+ * Unlike waitpid(), pthread_join() only waits until the worker thread
+ * has exited from the pthread point of view. The thread may still be
+ * visible through its pidfd for a short time afterwards, and fanotify
+ * creates the event pidfd when the event is read. Wait until the
+ * worker pidfd fdinfo reports Pid: -1 before reading the event so
+ * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
+ */
+ do {
+ read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
+ } while (thread_pidfd_fdinfo.pid != -1);
+
+ SAFE_CLOSE(pidfd);
+
+ return -1;
+ }
+
+ SAFE_CLOSE(pidfd);
+
+ return worker;
+}
+
+static void do_setup(void)
+{
int init_flags = FAN_REPORT_PIDFD;
- if (tst_variant) {
+ if (TST_VARIANT_FD_ERROR) {
fanotify_fd = -1;
fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
if (fd_error_unsupported)
@@ -131,6 +199,15 @@ static void do_setup(void)
init_flags |= FAN_REPORT_FD_ERROR;
}
+ if (TST_VARIANT_PIDFD_THREAD) {
+ fanotify_fd = -1;
+ thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
+ FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
+ if (thread_pidfd_unsupported)
+ return;
+ init_flags |= FAN_REPORT_TID;
+ }
+
SAFE_TOUCH(TEST_FILE, 0666, NULL);
/*
@@ -144,15 +221,6 @@ static void do_setup(void)
fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
TEST_FILE);
-
- pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
-
- self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
- if (self_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "pidfd=%d, failed to read pidfd fdinfo",
- pidfd);
- }
}
static void do_test(unsigned int num)
@@ -160,17 +228,29 @@ static void do_test(unsigned int num)
int i = 0, len;
struct test_case_t *tc = &test_cases[num];
int nopidfd_err = tc->want_pidfd_err ?
- (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
- int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
+ (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
+ int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
+ union {
+ pid_t pid;
+ pthread_t pthread_id;
+ } worker_id;
tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
- tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
+ TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
+ "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
+ (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
- if (fd_error_unsupported && tst_variant) {
+ if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
return;
}
+ if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
+ FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
+ thread_pidfd_unsupported);
+ return;
+ }
+
if (tc->remount_ro) {
/* SAFE_MOUNT fails to remount FUSE */
if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
@@ -182,14 +262,30 @@ static void do_test(unsigned int num)
}
/*
- * Generate the event in either self or a child process. Event
- * generation in a child process is done so that the FAN_NOPIDFD case
- * can be verified.
+ * Generate the event either in the current task or in another task.
+ * When trigger_in_child is set, the event can be generated by either
+ * a child process or a worker thread depending on the test variant.
+ * The want_pidfd_err field determines whether the event-generating
+ * task is still valid when the event is read.
*/
- if (tc->fork)
- do_fork();
- else
+ if (tc->trigger_in_child) {
+ if (TST_VARIANT_PIDFD_THREAD)
+ worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
+ else
+ worker_id.pid = do_fork(tc->want_pidfd_err);
+ } else {
+ /*
+ * Although the expected pid and the pid reported by fanotify are
+ * the same in this case, pidfds created with and without PIDFD_THREAD
+ * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
+ * pidfd fdinfo so that the fdinfo can be compared bitwise.
+ */
+ int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
+ read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
+ SAFE_CLOSE(pidfd);
+
generate_event();
+ }
/*
* Read all of the queued events into the provided event
@@ -208,7 +304,7 @@ static void do_test(unsigned int num)
while (i < len) {
struct fanotify_event_metadata *event;
struct fanotify_event_info_pidfd *info;
- struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
+ struct pidfd_fdinfo_t event_pidfd_fdinfo;
event = (struct fanotify_event_metadata *)&event_buf[i];
info = (struct fanotify_event_info_pidfd *)(event + 1);
@@ -288,39 +384,32 @@ static void do_test(unsigned int num)
* No pidfd errors occurred, continue with verifying pidfd
* fdinfo validity.
*/
- event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
- if (event_pidfd_fdinfo == NULL) {
- tst_brk(TBROK,
- "reading fdinfo for pidfd: %d "
- "describing pid: %u failed",
- info->pidfd,
- (unsigned int)event->pid);
- goto next_event;
- } else if (event_pidfd_fdinfo->pid != event->pid) {
+ read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
+ if (event_pidfd_fdinfo.pid != event->pid) {
tst_res(TFAIL,
"pidfd provided for incorrect pid "
"(expected pidfd for pid: %u, got pidfd for "
"pid: %u)",
(unsigned int)event->pid,
- (unsigned int)event_pidfd_fdinfo->pid);
+ (unsigned int)event_pidfd_fdinfo.pid);
goto next_event;
- } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
+ } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
sizeof(struct pidfd_fdinfo_t))) {
tst_res(TFAIL,
"pidfd fdinfo values for self and event differ "
"(expected pos: %d, flags: %x, mnt_id: %d, "
"pid: %d, ns_pid: %d, got pos: %d, "
"flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
- self_pidfd_fdinfo->pos,
- self_pidfd_fdinfo->flags,
- self_pidfd_fdinfo->mnt_id,
- self_pidfd_fdinfo->pid,
- self_pidfd_fdinfo->ns_pid,
- event_pidfd_fdinfo->pos,
- event_pidfd_fdinfo->flags,
- event_pidfd_fdinfo->mnt_id,
- event_pidfd_fdinfo->pid,
- event_pidfd_fdinfo->ns_pid);
+ expected_pidfd_fdinfo.pos,
+ expected_pidfd_fdinfo.flags,
+ expected_pidfd_fdinfo.mnt_id,
+ expected_pidfd_fdinfo.pid,
+ expected_pidfd_fdinfo.ns_pid,
+ event_pidfd_fdinfo.pos,
+ event_pidfd_fdinfo.flags,
+ event_pidfd_fdinfo.mnt_id,
+ event_pidfd_fdinfo.pid,
+ event_pidfd_fdinfo.ns_pid);
goto next_event;
} else {
tst_res(TPASS,
@@ -342,9 +431,20 @@ next_event:
if (info && info->pidfd >= 0)
SAFE_CLOSE(info->pidfd);
+ }
- if (event_pidfd_fdinfo)
- free(event_pidfd_fdinfo);
+ if (tc->trigger_in_child && !tc->want_pidfd_err) {
+ int status;
+ TST_CHECKPOINT_WAKE(0);
+ if (TST_VARIANT_PIDFD_THREAD) {
+ SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
+ if (status != 0)
+ tst_brk(TBROK, "worker thread terminated incorrectly");
+ } else {
+ SAFE_WAITPID(worker_id.pid, &status, 0);
+ if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
+ tst_brk(TBROK, "child process terminated incorrectly");
+ }
}
restore_rw_mount:
@@ -363,19 +463,17 @@ static void do_cleanup(void)
{
if (fanotify_fd >= 0)
SAFE_CLOSE(fanotify_fd);
-
- if (self_pidfd_fdinfo)
- free(self_pidfd_fdinfo);
}
static struct tst_test test = {
.setup = do_setup,
.test = do_test,
.tcnt = ARRAY_SIZE(test_cases),
- .test_variants = 2,
+ .test_variants = 4,
.cleanup = do_cleanup,
.all_filesystems = 1,
.needs_root = 1,
+ .needs_checkpoints = 1,
.mount_device = 1,
.mntpoint = MOUNT_PATH,
.forks_child = 1,
--
2.54.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [LTP] fanotify: fix crash when running multiple iterations
2026-05-27 19:50 ` [LTP] " AnonymeMeow
(?)
(?)
@ 2026-05-27 22:06 ` linuxtestproject.agent
-1 siblings, 0 replies; 21+ messages in thread
From: linuxtestproject.agent @ 2026-05-27 22:06 UTC (permalink / raw)
To: AnonymeMeow; +Cc: ltp
Hi AnonymeMeow,
On Thu, 28 May 2026 03:50:55 +0800, AnonymeMeow wrote:
> [PATCH 1/2] fanotify: fix crash when running multiple iterations
This commit fixes two unrelated crashes in two separate test files.
Consider splitting into two commits for cleaner history.
> +static void clean_upper_dir(void)
> +{
> + unsigned int i;
> +
> + SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
> + for (i = 0; i < ARRAY_SIZE(objects); i++)
> + SAFE_UNLINK(objects[i].path);
> + SAFE_UMOUNT(MOUNT_PATH);
> +}
SAFE_UNLINK is called unconditionally for all objects, including
DIR_PATH_ONE which is a directory. delete_objects() correctly uses
SAFE_RMDIR for directories. This works here only because overlayfs
whiteouts for directories are character devices, but it is fragile
and inconsistent with delete_objects(). Use is_dir the same way
delete_objects() does.
[...]
> --- [PATCH 2/2] ---
> fanotify: prepare tests for thread pidfd reporting
> +#include <pthread.h>
> #include "tst_test.h"
> #include "tst_safe_stdio.h"
> #include "tst_safe_macros.h"
> +#include "tst_safe_pthread.h"
> #include "lapi/pidfd.h"
> + *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
PIDFD_THREAD (added in Linux 6.9) has no fallback definition in
include/lapi/pidfd.h. This will fail to compile on systems with
kernel headers older than 6.9. Add:
#ifndef PIDFD_THREAD
# define PIDFD_THREAD O_EXCL
#endif
to include/lapi/pidfd.h.
[...]
> +static pthread_t do_pthread_create(int want_pidfd_err)
> {
> + int pidfd;
> pthread_t worker;
[...]
> + int status;
> + struct pidfd_fdinfo_t thread_pidfd_fdinfo;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_PTHREAD_JOIN(worker, (void **)&status);
> + if (status != 0)
status is int (4 bytes). SAFE_PTHREAD_JOIN writes a void * (8 bytes
on 64-bit) into &status, overflowing into adjacent stack memory.
Use void *status instead:
void *status;
SAFE_PTHREAD_JOIN(worker, &status);
if (status != NULL)
[...]
> + if (tc->trigger_in_child && !tc->want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + if (TST_VARIANT_PIDFD_THREAD) {
> + SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
> + if (status != 0)
Same int/void * mismatch as above.
---
Note:
The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.
Regards,
LTP AI Reviewer
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [LTP] [PATCH v2 1/2] fanotify: fix crash when running multiple iterations
2026-05-27 19:50 ` [LTP] " AnonymeMeow
` (2 preceding siblings ...)
(?)
@ 2026-05-28 13:03 ` Amir Goldstein
-1 siblings, 0 replies; 21+ messages in thread
From: Amir Goldstein @ 2026-05-28 13:03 UTC (permalink / raw)
To: AnonymeMeow; +Cc: jack, ltp, repnop
[reducing CC to LTP context]
On Wed, May 27, 2026 at 9:51 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> This commit fixes ./fanotify13 -i10 crash by restoring the deleted objects
> every time a test finishes.
>
> This commit also fixes ./fanotify21 -i10 crash by remounting the read-only
> mount (if the mount is remounted read-only during the test) back to
> read-write every time a test finishes.
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
>
> On 2026-05-27 09:23:12+02:00, Petr Vorel wrote:
> > @AnonymeMeow nit: if you put whole conversation after --- (below), it will not
> > be part of the commit message when one applies the patch. And here should be a
> > proper commit message.
>
> Thank you for pointing this out, I will be more mindful the next time
> (this time).
>
> > Also I found that fanotify21.c on the current master fails when running more
> > than 1 iteration:
>
> Thanks for testing my patch, I didn't know that the standalone test binary
> supports the -i option to specify the number of iterations. Running
> multiple iterations through kirk's -i option seems to work fine.
>
> I then tested all fanotify tests with the -i option and found that
> fanotify13 also crashed when running multiple iterations. I fixed both
> fanotify13 and fanotify21 in this patch. So now all fanotify tests pass
> with the -i option.
Thanks for testing! and for the fixes.
For fanotify21, I prefer a simpler fix (attached) which also removes the
FUSE limitation.
For fanotify13, see comments below.
Thanks,
Amir.
>
> With Best Regards,
> AnonymeMeow
>
> ---
> .../kernel/syscalls/fanotify/fanotify13.c | 20 +++++++++++++++++++
> .../kernel/syscalls/fanotify/fanotify21.c | 13 +++++++++++-
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index 76d40eaf7..540e0b483 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -137,6 +137,16 @@ static void delete_objects(void)
> }
> }
>
> +static void clean_upper_dir(void)
> +{
> + unsigned int i;
> +
> + SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
> + for (i = 0; i < ARRAY_SIZE(objects); i++)
> + SAFE_UNLINK(objects[i].path);
> + SAFE_UMOUNT(MOUNT_PATH);
> +}
Nah, this is "illegal" and results are undefined when cleaning stuff underneath
a mounted overlayfs.
> +
> static void get_object_stats(void)
> {
> unsigned int i;
> @@ -340,6 +350,15 @@ static void do_test(unsigned int number)
> "Did not get an expected event (expected: %llx)",
> event_set[i].expected_mask);
> }
> +
> + if (tc->mask & FAN_DELETE_SELF) {
> + if (tst_variant & 1) {
> + clean_upper_dir();
> + } else {
> + create_objects();
> + get_object_stats();
> + }
I am wondering why create_objects() does not work for overlayfs
I tested and I saw that it fails but did not yet understand why
looks like an unrelated issue. I will take a closer look.
The textbook solution is to do all the setup at the start of do_test()
but that's really ugly IMO.
Let me think about this some more...
> + }
> out:
> SAFE_CLOSE(fanotify_fd);
> }
> @@ -417,6 +436,7 @@ static void do_cleanup(void)
> if (ovl_bind_mounted)
> SAFE_UMOUNT(MOUNT_PATH);
> if (bind_mounted) {
> + delete_objects();
> SAFE_UMOUNT(MOUNT_PATH);
> SAFE_RMDIR(MOUNT_PATH);
> }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> index 340fb0018..2e3dbd4bd 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify21.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -199,7 +199,7 @@ static void do_test(unsigned int num)
> if (len < 0) {
> if (tc->remount_ro && !fd_err && errno == EROFS) {
> tst_res(TPASS, "cannot read event with rw fd from a ro fs");
> - return;
> + goto restore_rw_mount;
> }
> tst_brk(TBROK | TERRNO, "reading fanotify events failed");
> } else if (tc->remount_ro && !fd_err) {
> @@ -346,6 +346,17 @@ next_event:
> if (event_pidfd_fdinfo)
> free(event_pidfd_fdinfo);
> }
> +
> +restore_rw_mount:
> + if (tc->remount_ro) {
> + /* SAFE_MOUNT fails to remount FUSE */
> + if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> + MS_REMOUNT, NULL) != 0) {
> + tst_brk(TFAIL,
> + "filesystem %s failed to remount read-write",
> + tst_device->fs_type);
> + }
> + }
> }
I think my attached version is simpler/cleaner.
Thanks,
Amir.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [LTP] fanotify: prepare tests for thread pidfd reporting
2026-05-27 6:40 ` [LTP] " AnonymeMeow
(?)
(?)
@ 2026-05-27 9:53 ` linuxtestproject.agent
-1 siblings, 0 replies; 21+ messages in thread
From: linuxtestproject.agent @ 2026-05-27 9:53 UTC (permalink / raw)
To: AnonymeMeow; +Cc: ltp
Hi AnonymeMeow,
On Wed May 27 2026, AnonymeMeow wrote:
> fanotify: prepare tests for thread pidfd reporting
Commit message body is an email exchange transcript rather than a clean
description of what the patch does and why. Please rewrite it as a
standalone commit message.
---
> diff --git a/include/lapi/pidfd.h b/include/lapi/pidfd.h
PIDFD_THREAD is used in fanotify21.c but is not defined in
include/lapi/pidfd.h. On systems with kernel headers older than 6.9
this will fail to compile. Add a fallback:
#ifndef PIDFD_THREAD
# define PIDFD_THREAD O_EXCL
#endif
---
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/...
[...]
> +unsigned int needs_thread_pidfd;
Other boolean-like fields in test_case_t use int. Make this int for
consistency.
[...]
The file-level docstring says the test "ensures that the fanotify(7) API
returns an expected error code when provided an invalid initialization
flag". The new third test case behaves differently depending on kernel
support (may expect success). Update the docstring.
---
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/...
[...]
> +if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> +tst_brk(TBROK, "child process terminated incorrectly");
> +
Trailing whitespace on the blank line above.
[...]
> +static pthread_t do_pthread_create(int want_pidfd_err)
> +{
[...]
> +SAFE_PTHREAD_JOIN(worker, (void **)&status);
> +if (status != 0)
status is int (4 bytes) but pthread_join writes a void* (8 bytes on
64-bit) at that address, corrupting the stack. Use NULL since the return
value is not needed:
SAFE_PTHREAD_JOIN(worker, NULL);
[...]
> +if (tc->trigger_in_child && !tc->want_pidfd_err) {
> +int status;
> +TST_CHECKPOINT_WAKE(0);
> +if (TST_VARIANT_PIDFD_THREAD) {
> +SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
> +if (status != 0)
Same bug: (void **)&status writes 8 bytes to a 4-byte int. Use NULL or
a void* variable here too.
---
Needs revision:
- Add PIDFD_THREAD fallback to include/lapi/pidfd.h
- Fix SAFE_PTHREAD_JOIN (void **)&status type mismatch in both call sites
- Clean up commit message
- Update fanotify20 docstring to cover the new kernel-conditional test case
- Fix trailing whitespace and needs_thread_pidfd type
---
Note:
The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.
Regards,
LTP AI Reviewer
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [LTP] [PATCH] fanotify: prepare tests for thread pidfd reporting
2026-05-27 6:40 ` [LTP] " AnonymeMeow
` (2 preceding siblings ...)
(?)
@ 2026-05-27 19:54 ` Amir Goldstein
-1 siblings, 0 replies; 21+ messages in thread
From: Amir Goldstein @ 2026-05-27 19:54 UTC (permalink / raw)
To: AnonymeMeow; +Cc: repnop, jack, ltp
On Wed, May 27, 2026 at 8:41 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> On 2026-05-25 12:04:32+02:00, Amir Goldstein wrote:
> > On Sun, May 24, 2026 at 12:25 PM AnonymeMeow <anonymemeow@gmail.com> wrote:
> >
> > > The FAN_REPORT_PIDFD and FAN_REPORT_TID flags used to be mutually
> > > exclusive because by the time the pidfd support was introduced to
> > > fanotify, pidfds could only be created for thread group leaders. Now
> > > that the pidfd API supports thread-specific pidfds via PIDFD_THREAD,
> > > this restriction can be lifted.
> > >
> > > This patch allows listeners using FAN_REPORT_PIDFD | FAN_REPORT_TID
> > > to receive the pidfd referring to the thread that triggered the
> > > event.
> > >
> > > Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> >
> > Regarding the legal standpoint, as far as I can tell, commit d4563201f33a0
> > ("Documentation: simplify and clarify DCO contribution example language")
> > explicitly allowed pseudonyms, so although unorthodox, this seems to be legit.
> >
> > In any case, the contributing itself seems legit to me, so
> >
> > Reviewed-by: Amir Goldstein <amir73il@gmail.com>
> >
> > But before merging this, AnonymeMeow, since it is your first contribution
> > to the Linux kernel (right?) as well as a "I am not a robot" challenge
> > I would like to first see that the tests have been dealt with.
> >
> > The LTP test fanotify20 explicitly checks that this flag combination is not
> > allowed. Need to discuss an acceptable solution with LTP developers -
> > remove this check depending on kernel version or whatever they choose.
> >
> > The LTP test fanotify21 tests the FAN_REPORT_PIDFD functionality.
> > Please add two new variants for REPORT_TID:
> > .test_variants = 4,
> >
> > #define TST_VARIANT_FD_ERROR (tst_variant & 1)
> > #define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
> >
> > The existing checks if (tst_variant) become if (TST_VARIANT_FD_ERROR)
> > so bit 0 represents the FAN_REPORT_FD_ERROR variation.
> >
> > in do_setup() need to use if (TST_VARIANT_PIDFD_THREAD)
> > to add FAN_REPORT_TID and to use PIDFD_THREAD for pidfd.
> >
> > Follow the footsteps of fd_error_unsupported to skip the
> > TST_VARIANT_PIDFD_THREAD variants on kernels where the flag
> > combination is not supported.
> >
> > Good luck,
> > Amir.
>
> Thanks for the helpful and detailed guidance on how to extend the tests!
> I've updated the test cases according to your suggestions.
[cc Cyril and remove lkml and fsdevel]
The remaining CC is the correct crowd for the LTP patches.
Also please send a patch series with a cover letter (one more new
thing to learn ;))
instead of one big patch.
One patch to fix fanotify20.
Possibly one patch to fix existing problem that Petr reported in fanotify21.
Then several patches for the new test (see below)
>
> After applying my previous patch, the LTP fanotify20 test needs to probe
> whether the running kernel supports thread pidfd reporting. I didn't use
> kernel version check because the LTP documentation notes that kernel
> versions do not always correspond to a well-defined feature set due to
> distribution backports, and recommends runtime feature checks. I probe
> whether fanotify_init() succeeds with FAN_REPORT_PIDFD | FAN_REPORT_TID
> as a workaround. The first test case uses exactly the same flags, but it
> still verifies the return value and errno, so I am not sure whether this
> should be considered duplicate logic. I also added one test case that
> passes all relevant FAN_REPORT_* flags and verifies whether
> fanotify_init() behaves as expected. The expected result depends on the
> previous probe result.
>
> Also, I added one test case and two test variants to LTP test
> fanotify21. The new test case verifies that fanotify reports the correct
> pidfd for the event generated by a child process or a worker thread
> created by the main thread. This is especially useful for the thread
> pidfd test variants, because it checks that the pidfd reported by
> fanotify really refers to the event-generating thread rather than to
> the thread-group leader. The two new test variants extend the existing
> ones by adding the thread pidfd mode, as you suggested.
>
> I ran all fanotify-related LTP tests on both a vanilla Linux 7.1-rc5
> kernel and a Linux 7.1-rc5 kernel with my patch applied. Both kernels
> passed all the tests. They were both built with the config file from the
> official Arch Linux package repo. More specifically, both kernels passed
> all three test cases in fanotify20, with no skips. The test cases expect
> different results from the two kernels, which is the expected behavior.
> For fanotify21, the patched kernel passed all test cases in all test
> variants. On the unpatched kernel, all test cases passed for variants
> without TST_VARIANT_PIDFD_THREAD, while all test cases in variants with
> TST_VARIANT_PIDFD_THREAD were skipped, which is also the expected
> behavior.
Up to here this is cover letter material.
You should probably refer to your kernel patch on lore from the cover letter:
https://lore.kernel.org/linux-fsdevel/20260524102439.44642-1-anonymemeow@gmail.com/
>
> Again, thanks for the detailed guidance on this!
>
> With Best Regards,
> AnonymeMeow
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
> testcases/kernel/syscalls/fanotify/Makefile | 2 +-
> .../kernel/syscalls/fanotify/fanotify20.c | 24 +-
> .../kernel/syscalls/fanotify/fanotify21.c | 224 +++++++++++++-----
> 3 files changed, 184 insertions(+), 66 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/Makefile b/testcases/kernel/syscalls/fanotify/Makefile
> index 3628094ba..b20bb50e9 100644
> --- a/testcases/kernel/syscalls/fanotify/Makefile
> +++ b/testcases/kernel/syscalls/fanotify/Makefile
> @@ -2,7 +2,7 @@
> # Copyright (c) Jan Kara <jack@suse.cz>, 2013
>
> top_srcdir ?= ../../../..
> -fanotify11: CFLAGS+=-pthread
> +fanotify11 fanotify21: CFLAGS+=-pthread
> include $(top_srcdir)/include/mk/testcases.mk
>
> include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index b32ecf6aa..5d77b485c 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -28,19 +28,27 @@
> #define FLAGS_DESC(x) .flags = x, .desc = #x
>
> static int fd;
> +static int thread_pidfd_unsupported;
>
> static struct test_case_t {
> unsigned int flags;
> char *desc;
> int exp_errno;
> + unsigned int needs_thread_pidfd;
> } test_cases[] = {
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID),
> .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> },
Unless LTP developers think otherwise, I would just remove this test case.
> {
> FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> },
> + {
> + FLAGS_DESC(FAN_REPORT_PIDFD | FAN_REPORT_TID | FAN_REPORT_FID | FAN_REPORT_DFID_NAME),
> + .exp_errno = EINVAL,
> + .needs_thread_pidfd = 1,
> + },
TBH I do not see much value in adding this test case.
> };
>
> static void do_setup(void)
> @@ -51,17 +59,29 @@ static void do_setup(void)
> */
> REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_PIDFD,
> MOUNT_PATH);
> +
> + /*
> + * Check whether the kernel supports FAN_REPORT_PIDFD in combination
> + * with FAN_REPORT_TID. Test cases with the needs_thread_pidfd field
> + * set expect different errno values depending on whether this
> + * combination is supported.
> + */
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, MOUNT_PATH);
> }
>
This makes no sense to me.
What's the point is a test that checks the flag combination
FAN_REPORT_PIDFD | FAN_REPORT_TID
and if it succeeds/fails, tests it again and verifies the same result
(well it validates the errno EINVAL)
Not much value for a test IMO.
I don't know.
For all I care (if others do not object) you can repurpose the test
fanotify20 from a test which is expected to fail with the flag combination
FAN_REPORT_PIDFD | FAN_REPORT_TID to a test which verifies that
the combination works as expected.
Let's see what the others think.
> static void do_test(unsigned int i)
> {
> struct test_case_t *tc = &test_cases[i];
>
> - tst_res(TINFO, "Test %s on %s", tc->exp_errno ? "fail" : "pass",
> + int exp_errno = tc->needs_thread_pidfd && !thread_pidfd_unsupported ?
> + 0 : tc->exp_errno;
> +
> + tst_res(TINFO, "Test %s on %s", exp_errno ? "fail" : "pass",
> tc->desc);
>
> TST_EXP_FD_OR_FAIL(fd = fanotify_init(tc->flags, O_RDONLY),
> - tc->exp_errno);
> + exp_errno);
>
> if (fd > 0)
> SAFE_CLOSE(fd);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> index 340fb0018..4629860da 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify21.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -20,9 +20,11 @@
> #include <stdlib.h>
> #include <string.h>
> #include <sys/mount.h>
> +#include <pthread.h>
> #include "tst_test.h"
> #include "tst_safe_stdio.h"
> #include "tst_safe_macros.h"
> +#include "tst_safe_pthread.h"
> #include "lapi/pidfd.h"
>
> #ifdef HAVE_SYS_FANOTIFY_H
> @@ -42,7 +44,7 @@ struct pidfd_fdinfo_t {
>
> static struct test_case_t {
> char *name;
> - int fork;
> + int trigger_in_child;
> int want_pidfd_err;
> int remount_ro;
> } test_cases[] = {
> @@ -52,6 +54,12 @@ static struct test_case_t {
> 0,
> 0,
> },
> + {
> + "return a valid pidfd for event created by child",
> + 1,
> + 0,
> + 0,
> + },
> {
> "return invalid pidfd for event created by terminated child",
> 1,
> @@ -68,16 +76,17 @@ static struct test_case_t {
>
> static int fanotify_fd;
> static char event_buf[BUF_SZ];
> -static struct pidfd_fdinfo_t *self_pidfd_fdinfo;
> +static struct pidfd_fdinfo_t expected_pidfd_fdinfo;
>
> static int fd_error_unsupported;
> +static int thread_pidfd_unsupported;
> +
> +#define TST_VARIANT_FD_ERROR (tst_variant & 1)
> +#define TST_VARIANT_PIDFD_THREAD (tst_variant & 2)
>
> -static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> +static void read_pidfd_fdinfo(int pidfd, struct pidfd_fdinfo_t *pidfd_fdinfo)
> {
> char *fdinfo_path;
> - struct pidfd_fdinfo_t *pidfd_fdinfo;
> -
> - pidfd_fdinfo = SAFE_MALLOC(sizeof(struct pidfd_fdinfo_t));
>
> SAFE_ASPRINTF(&fdinfo_path, "/proc/self/fdinfo/%d", pidfd);
> SAFE_FILE_LINES_SCANF(fdinfo_path, "pos: %d", &pidfd_fdinfo->pos);
> @@ -87,8 +96,6 @@ static struct pidfd_fdinfo_t *read_pidfd_fdinfo(int pidfd)
> SAFE_FILE_LINES_SCANF(fdinfo_path, "NSpid: %d", &pidfd_fdinfo->ns_pid);
>
> free(fdinfo_path);
> -
> - return pidfd_fdinfo;
> }
>
> static void generate_event(void)
> @@ -100,30 +107,91 @@ static void generate_event(void)
> SAFE_CLOSE(fd);
> }
>
> -static void do_fork(void)
> +static pid_t do_fork(int want_pidfd_err)
> {
> - int status;
> + int pidfd;
> pid_t child;
>
> child = SAFE_FORK();
> if (child == 0) {
> SAFE_CLOSE(fanotify_fd);
> generate_event();
> + TST_CHECKPOINT_WAIT(0);
> exit(EXIT_SUCCESS);
> }
>
> - SAFE_WAITPID(child, &status, 0);
> - if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> - tst_brk(TBROK,
> - "child process terminated incorrectly");
> + pidfd = SAFE_PIDFD_OPEN(child, 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> + if (want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_WAITPID(child, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> +
> + return -1;
> + }
> +
> + return child;
> }
>
> -static void do_setup(void)
> +static void *thread_generate_event(void *arg)
> +{
> + *(int *)arg = SAFE_PIDFD_OPEN(gettid(), PIDFD_THREAD);
> + TST_CHECKPOINT_WAKE(0);
> +
> + generate_event();
> + TST_CHECKPOINT_WAIT(0);
> + pthread_exit(0);
> +}
> +
> +static pthread_t do_pthread_create(int want_pidfd_err)
> {
> int pidfd;
> + pthread_t worker;
> +
> + SAFE_PTHREAD_CREATE(&worker, NULL, thread_generate_event, &pidfd);
> +
> + TST_CHECKPOINT_WAIT(0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> +
> + if (want_pidfd_err) {
> + int status;
> + struct pidfd_fdinfo_t thread_pidfd_fdinfo;
> + TST_CHECKPOINT_WAKE(0);
> + SAFE_PTHREAD_JOIN(worker, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> +
> + /*
> + * Unlike waitpid(), pthread_join() only waits until the worker thread
> + * has exited from the pthread point of view. The thread may still be
> + * visible through its pidfd for a short time afterwards, and fanotify
> + * creates the event pidfd when the event is read. Wait until the
> + * worker pidfd fdinfo reports Pid: -1 before reading the event so
> + * that fanotify reports ESRCH/FAN_NOPIDFD instead of a pidfd.
> + */
> + do {
> + read_pidfd_fdinfo(pidfd, &thread_pidfd_fdinfo);
> + } while (thread_pidfd_fdinfo.pid != -1);
> +
> + SAFE_CLOSE(pidfd);
> +
> + return -1;
> + }
> +
> + SAFE_CLOSE(pidfd);
> +
> + return worker;
> +}
> +
> +static void do_setup(void)
> +{
> int init_flags = FAN_REPORT_PIDFD;
>
> - if (tst_variant) {
> + if (TST_VARIANT_FD_ERROR) {
> fanotify_fd = -1;
> fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
> if (fd_error_unsupported)
> @@ -131,6 +199,15 @@ static void do_setup(void)
> init_flags |= FAN_REPORT_FD_ERROR;
> }
>
> + if (TST_VARIANT_PIDFD_THREAD) {
> + fanotify_fd = -1;
> + thread_pidfd_unsupported = fanotify_init_flags_supported_on_fs(
> + FAN_REPORT_PIDFD | FAN_REPORT_TID, ".");
> + if (thread_pidfd_unsupported)
> + return;
> + init_flags |= FAN_REPORT_TID;
> + }
> +
> SAFE_TOUCH(TEST_FILE, 0666, NULL);
>
> /*
> @@ -144,15 +221,6 @@ static void do_setup(void)
> fanotify_fd = SAFE_FANOTIFY_INIT(init_flags, O_RDWR);
> SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD,
> TEST_FILE);
> -
> - pidfd = SAFE_PIDFD_OPEN(getpid(), 0);
> -
> - self_pidfd_fdinfo = read_pidfd_fdinfo(pidfd);
> - if (self_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "pidfd=%d, failed to read pidfd fdinfo",
> - pidfd);
> - }
> }
>
> static void do_test(unsigned int num)
> @@ -160,17 +228,29 @@ static void do_test(unsigned int num)
> int i = 0, len;
> struct test_case_t *tc = &test_cases[num];
> int nopidfd_err = tc->want_pidfd_err ?
> - (tst_variant ? -ESRCH : FAN_NOPIDFD) : 0;
> - int fd_err = (tc->remount_ro && tst_variant) ? -EROFS : 0;
> + (TST_VARIANT_FD_ERROR ? -ESRCH : FAN_NOPIDFD) : 0;
> + int fd_err = (tc->remount_ro && TST_VARIANT_FD_ERROR) ? -EROFS : 0;
> + union {
> + pid_t pid;
> + pthread_t pthread_id;
> + } worker_id;
>
> tst_res(TINFO, "Test #%d.%d: %s %s", num, tst_variant, tc->name,
> - tst_variant ? "(FAN_REPORT_FD_ERROR)" : "");
> + TST_VARIANT_FD_ERROR ? (TST_VARIANT_PIDFD_THREAD ?
> + "(FAN_REPORT_FD_ERROR, FAN_REPORT_TID)" : "(FAN_REPORT_FD_ERROR)") :
> + (TST_VARIANT_PIDFD_THREAD ? "(FAN_REPORT_TID)" : ""));
>
> - if (fd_error_unsupported && tst_variant) {
> + if (fd_error_unsupported && TST_VARIANT_FD_ERROR) {
> FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_FD_ERROR, fd_error_unsupported);
> return;
> }
>
> + if (thread_pidfd_unsupported && TST_VARIANT_PIDFD_THREAD) {
> + FANOTIFY_INIT_FLAGS_ERR_MSG(FAN_REPORT_PIDFD | FAN_REPORT_TID,
> + thread_pidfd_unsupported);
> + return;
> + }
> +
> if (tc->remount_ro) {
> /* SAFE_MOUNT fails to remount FUSE */
> if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> @@ -182,14 +262,30 @@ static void do_test(unsigned int num)
> }
>
> /*
> - * Generate the event in either self or a child process. Event
> - * generation in a child process is done so that the FAN_NOPIDFD case
> - * can be verified.
> + * Generate the event either in the current task or in another task.
> + * When trigger_in_child is set, the event can be generated by either
> + * a child process or a worker thread depending on the test variant.
> + * The want_pidfd_err field determines whether the event-generating
> + * task is still valid when the event is read.
> */
> - if (tc->fork)
> - do_fork();
> - else
> + if (tc->trigger_in_child) {
> + if (TST_VARIANT_PIDFD_THREAD)
> + worker_id.pthread_id = do_pthread_create(tc->want_pidfd_err);
> + else
> + worker_id.pid = do_fork(tc->want_pidfd_err);
This misses test coverage
We are not testing do_pthread_create() without FAN_REPORT_TID
and we are not testing that do_fork with FAN_REPORT_TID
> + } else {
> + /*
> + * Although the expected pid and the pid reported by fanotify are
> + * the same in this case, pidfds created with and without PIDFD_THREAD
> + * flag have different fdinfo flags. Use PIDFD_THREAD for the expected
> + * pidfd fdinfo so that the fdinfo can be compared bitwise.
> + */
> + int pidfd = SAFE_PIDFD_OPEN(gettid(), TST_VARIANT_PIDFD_THREAD ? PIDFD_THREAD : 0);
> + read_pidfd_fdinfo(pidfd, &expected_pidfd_fdinfo);
> + SAFE_CLOSE(pidfd);
> +
> generate_event();
> + }
>
> /*
> * Read all of the queued events into the provided event
> @@ -208,7 +304,7 @@ static void do_test(unsigned int num)
> while (i < len) {
> struct fanotify_event_metadata *event;
> struct fanotify_event_info_pidfd *info;
> - struct pidfd_fdinfo_t *event_pidfd_fdinfo = NULL;
> + struct pidfd_fdinfo_t event_pidfd_fdinfo;
>
> event = (struct fanotify_event_metadata *)&event_buf[i];
> info = (struct fanotify_event_info_pidfd *)(event + 1);
> @@ -288,39 +384,32 @@ static void do_test(unsigned int num)
> * No pidfd errors occurred, continue with verifying pidfd
> * fdinfo validity.
> */
> - event_pidfd_fdinfo = read_pidfd_fdinfo(info->pidfd);
> - if (event_pidfd_fdinfo == NULL) {
> - tst_brk(TBROK,
> - "reading fdinfo for pidfd: %d "
> - "describing pid: %u failed",
> - info->pidfd,
> - (unsigned int)event->pid);
> - goto next_event;
> - } else if (event_pidfd_fdinfo->pid != event->pid) {
> + read_pidfd_fdinfo(info->pidfd, &event_pidfd_fdinfo);
> + if (event_pidfd_fdinfo.pid != event->pid) {
> tst_res(TFAIL,
> "pidfd provided for incorrect pid "
> "(expected pidfd for pid: %u, got pidfd for "
> "pid: %u)",
> (unsigned int)event->pid,
> - (unsigned int)event_pidfd_fdinfo->pid);
> + (unsigned int)event_pidfd_fdinfo.pid);
> goto next_event;
> - } else if (memcmp(event_pidfd_fdinfo, self_pidfd_fdinfo,
> + } else if (memcmp(&event_pidfd_fdinfo, &expected_pidfd_fdinfo,
> sizeof(struct pidfd_fdinfo_t))) {
> tst_res(TFAIL,
> "pidfd fdinfo values for self and event differ "
> "(expected pos: %d, flags: %x, mnt_id: %d, "
> "pid: %d, ns_pid: %d, got pos: %d, "
> "flags: %x, mnt_id: %d, pid: %d, ns_pid: %d",
> - self_pidfd_fdinfo->pos,
> - self_pidfd_fdinfo->flags,
> - self_pidfd_fdinfo->mnt_id,
> - self_pidfd_fdinfo->pid,
> - self_pidfd_fdinfo->ns_pid,
> - event_pidfd_fdinfo->pos,
> - event_pidfd_fdinfo->flags,
> - event_pidfd_fdinfo->mnt_id,
> - event_pidfd_fdinfo->pid,
> - event_pidfd_fdinfo->ns_pid);
> + expected_pidfd_fdinfo.pos,
> + expected_pidfd_fdinfo.flags,
> + expected_pidfd_fdinfo.mnt_id,
> + expected_pidfd_fdinfo.pid,
> + expected_pidfd_fdinfo.ns_pid,
> + event_pidfd_fdinfo.pos,
> + event_pidfd_fdinfo.flags,
> + event_pidfd_fdinfo.mnt_id,
> + event_pidfd_fdinfo.pid,
> + event_pidfd_fdinfo.ns_pid);
> goto next_event;
> } else {
> tst_res(TPASS,
> @@ -342,9 +431,20 @@ next_event:
>
> if (info && info->pidfd >= 0)
> SAFE_CLOSE(info->pidfd);
> + }
>
> - if (event_pidfd_fdinfo)
> - free(event_pidfd_fdinfo);
> + if (tc->trigger_in_child && !tc->want_pidfd_err) {
> + int status;
> + TST_CHECKPOINT_WAKE(0);
> + if (TST_VARIANT_PIDFD_THREAD) {
> + SAFE_PTHREAD_JOIN(worker_id.pthread_id, (void **)&status);
> + if (status != 0)
> + tst_brk(TBROK, "worker thread terminated incorrectly");
> + } else {
> + SAFE_WAITPID(worker_id.pid, &status, 0);
> + if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
> + tst_brk(TBROK, "child process terminated incorrectly");
> + }
> }
> }
>
> @@ -352,19 +452,17 @@ static void do_cleanup(void)
> {
> if (fanotify_fd >= 0)
> SAFE_CLOSE(fanotify_fd);
> -
> - if (self_pidfd_fdinfo)
> - free(self_pidfd_fdinfo);
> }
>
> static struct tst_test test = {
> .setup = do_setup,
> .test = do_test,
> .tcnt = ARRAY_SIZE(test_cases),
> - .test_variants = 2,
> + .test_variants = 4,
> .cleanup = do_cleanup,
> .all_filesystems = 1,
> .needs_root = 1,
> + .needs_checkpoints = 1,
> .mount_device = 1,
> .mntpoint = MOUNT_PATH,
> .forks_child = 1,
> --
> 2.54.0
>
This patch is very large and hard to review.
One option is to start with prep patches (like return event_pidfd_fdinfo
by value use help vars etc) and keep the new test variants patch as small
as possible.
But TBH I think I gave you bad advice.
The idea of test_variants is that the MOST of the code is the same
but it looks like 90% of the code if branched by this variant.
In this case, forking the test itself (to fanotify20 or another number)
is a better idea.
fanotify21 is testing FAN_REPORT_PIDFD with/without fork.
It still needs the 2 new TST_VARIANT_PIDFD_THREAD variants,
with FAN_REPORT_TID, but not with pthread_create.
The new test (say fanotify20) only needs to test FAN_REPORT_PIDFD
with pthread_create (which is most of the new code) and it also needs
the TST_VARIANT_PIDFD_THREAD variants, but in this case the
expected reported pidfd would be different between the variants.
Thanks,
Amir.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 21+ messages in thread