LinuxLists.cc - [PATCH v3 0/9] Test the new fanotify FAN_FS

[permalink] [raw]

Subject: [PATCH v3 2/9] syscalls: fanotify: Add macro to require specific events

Add a helper for tests to fail if an event is not available in the
kernel. Since some events only work with REPORT_FID or a specific
class, update the verifier to allow those to be specified.

Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

---
Changes since v1:
- Use SAFE_FANOTIFY_INIT instead of open coding. (Amir)
- Use FAN_CLASS_NOTIF for fanotify12 testcase. (Amir)
---
testcases/kernel/syscalls/fanotify/fanotify.h | 17 ++++++++++++++---
testcases/kernel/syscalls/fanotify/fanotify03.c | 4 ++--
testcases/kernel/syscalls/fanotify/fanotify10.c | 3 ++-
testcases/kernel/syscalls/fanotify/fanotify12.c | 3 ++-
4 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index c67db3117e29..820073709571 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -266,14 +266,16 @@ static inline void require_fanotify_access_permissions_supported_by_kernel(void)
SAFE_CLOSE(fd);
}

-static inline int fanotify_events_supported_by_kernel(uint64_t mask)
+static inline int fanotify_events_supported_by_kernel(uint64_t mask,
+ unsigned int init_flags,
+ unsigned int mark_flags)
{
int fd;
int rval = 0;

- fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+ fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);

- if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
+ if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
if (errno == EINVAL) {
rval = -1;
} else {
@@ -378,4 +380,13 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
fanotify_mark_supported_by_kernel(mark_type)); \
} while (0)

+#define REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(init_flags, mark_type, mask, fname) do { \
+ if (mark_type) \
+ REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type); \
+ if (init_flags) \
+ REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(init_flags, fname); \
+ fanotify_init_flags_err_msg(#mask, __FILE__, __LINE__, tst_brk_, \
+ fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
+} while (0)
+
#endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 26d17e64d1f5..2081f0bd1b57 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -323,8 +323,8 @@ static void setup(void)
require_fanotify_access_permissions_supported_by_kernel();

filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
- exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
-
+ exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM,
+ FAN_CLASS_CONTENT, 0);
sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
SAFE_FILE_PRINTF(fname, "1");

diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 92e4d3ff3054..0fa9d1f4f7e4 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -509,7 +509,8 @@ cleanup:

static void setup(void)
{
- exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
+ exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
+ FAN_CLASS_CONTENT, 0);
filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
MOUNT_PATH);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index 76f1aca77615..c77dbfd8c23d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -222,7 +222,8 @@ cleanup:

static void do_setup(void)
{
- exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
+ exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
+ FAN_CLASS_NOTIF, 0);

sprintf(fname, "fname_%d", getpid());
SAFE_FILE_PRINTF(fname, "1");
--
2.33.0

2021-10-29 21:19:04

[permalink] [raw]

Subject: [PATCH v3 3/9] syscalls/fanotify21: Introduce FAN_FS_ERROR test

fanotify21 is a new test validating the FAN_FS_ERROR file system error
event. This adds some basic structure for the next patches and a single
test of error reporting during filesystem abort

The strategy for error reporting testing in fanotify21 goes like this:

- Generate a broken filesystem
- Start FAN_FS_ERROR monitoring group
- Make the file system notice the error through ordinary operations
- Observe the event generated

Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

---
Changes since v1:
- Move defines to header file.
- Move fanotify_mark(2) to do_test (Amir)
- Merge abort test here
---
testcases/kernel/syscalls/fanotify/.gitignore | 1 +
testcases/kernel/syscalls/fanotify/fanotify.h | 3 +
.../kernel/syscalls/fanotify/fanotify21.c | 141 ++++++++++++++++++
3 files changed, 145 insertions(+)
create mode 100644 testcases/kernel/syscalls/fanotify/fanotify21.c

diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index 9554b16b196e..79ad184d578b 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -17,4 +17,5 @@
/fanotify17
/fanotify18
/fanotify19
+/fanotify21
/fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 820073709571..99b898554ede 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -124,6 +124,9 @@ static inline int safe_fanotify_mark(const char *file, const int lineno,
#ifndef FAN_OPEN_EXEC_PERM
#define FAN_OPEN_EXEC_PERM 0x00040000
#endif
+#ifndef FAN_FS_ERROR
+#define FAN_FS_ERROR 0x00008000
+#endif

/* Flags required for unprivileged user group */
#define FANOTIFY_REQUIRED_USER_INIT_FLAGS (FAN_REPORT_FID)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
new file mode 100644
index 000000000000..9ef687442b7c
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2021 Collabora Ltd.
+ *
+ * Author: Gabriel Krisman Bertazi <[email protected]>
+ * Based on previous work by Amir Goldstein <[email protected]>
+ */
+
+/*\
+ * [Description]
+ * Check fanotify FAN_ERROR_FS events triggered by intentionally
+ * corrupted filesystems:
+ *
+ * - Generate a broken filesystem
+ * - Start FAN_FS_ERROR monitoring group
+ * - Make the file system notice the error through ordinary operations
+ * - Observe the event generated
+ */
+
+#define _GNU_SOURCE
+#include "config.h"
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <errno.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/syscall.h>
+#include "tst_test.h"
+#include <sys/fanotify.h>
+#include <sys/types.h>
+#include <fcntl.h>
+
+#ifdef HAVE_SYS_FANOTIFY_H
+#include "fanotify.h"
+
+#define BUF_SIZE 256
+static char event_buf[BUF_SIZE];
+int fd_notify;
+
+#define MOUNT_PATH "test_mnt"
+
+static void trigger_fs_abort(void)
+{
+ SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
+ MS_REMOUNT|MS_RDONLY, "abort");
+}
+
+static struct test_case {
+ char *name;
+ void (*trigger_error)(void);
+} testcases[] = {
+ {
+ .name = "Trigger abort",
+ .trigger_error = &trigger_fs_abort,
+ },
+};
+
+int check_error_event_metadata(struct fanotify_event_metadata *event)
+{
+ int fail = 0;
+
+ if (event->mask != FAN_FS_ERROR) {
+ fail++;
+ tst_res(TFAIL, "got unexpected event %llx",
+ (unsigned long long)event->mask);
+ }
+
+ if (event->fd != FAN_NOFD) {
+ fail++;
+ tst_res(TFAIL, "Weird FAN_FD %llx",
+ (unsigned long long)event->mask);
+ }
+ return fail;
+}
+
+void check_event(char *buf, size_t len, const struct test_case *ex)
+{
+ struct fanotify_event_metadata *event =
+ (struct fanotify_event_metadata *) buf;
+
+ if (len < FAN_EVENT_METADATA_LEN) {
+ tst_res(TFAIL, "No event metadata found");
+ return;
+ }
+
+ if (check_error_event_metadata(event))
+ return;
+
+ tst_res(TPASS, "Successfully received: %s", ex->name);
+}
+
+static void do_test(unsigned int i)
+{
+ const struct test_case *tcase = &testcases[i];
+ size_t read_len;
+
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD|FAN_MARK_FILESYSTEM,
+ FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
+
+ tcase->trigger_error();
+
+ read_len = SAFE_READ(0, fd_notify, event_buf, BUF_SIZE);
+
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE|FAN_MARK_FILESYSTEM,
+ FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
+
+ check_event(event_buf, read_len, tcase);
+}
+
+static void setup(void)
+{
+ REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_NOTIF|FAN_REPORT_FID,
+ FAN_MARK_FILESYSTEM,
+ FAN_FS_ERROR, ".");
+
+ fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF|FAN_REPORT_FID,
+ O_RDONLY);
+}
+
+static void cleanup(void)
+{
+ if (fd_notify > 0)
+ SAFE_CLOSE(fd_notify);
+}
+
+static struct tst_test test = {
+ .test = do_test,
+ .tcnt = ARRAY_SIZE(testcases),
+ .setup = setup,
+ .cleanup = cleanup,
+ .mount_device = 1,
+ .mntpoint = MOUNT_PATH,
+ .all_filesystems = 0,
+ .needs_root = 1,
+ .dev_fs_type = "ext4"
+};
+
+#else
+ TST_TEST_TCONF("system doesn't have required fanotify support");
+#endif
--
2.33.0

2021-10-29 21:19:06

[permalink] [raw]

Subject: [PATCH v3 5/9] syscalls/fanotify21: Validate incoming FID in FAN_FS_ERROR

Verify the FID provided in the event. If the FH has size 0, this is
assumed to be a superblock error (i.e. null FH).

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

---
Changes since v2:
- Move FILEID_INVALID define to header file (Amir)
Changes since v1:
- Move defines to header file.
- Use 0-len FH for sb error
---
testcases/kernel/syscalls/fanotify/fanotify.h | 9 +++
.../kernel/syscalls/fanotify/fanotify21.c | 60 +++++++++++++++++++
2 files changed, 69 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 4294799fe86d..f8c39aa490ae 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -234,6 +234,11 @@ static inline void fanotify_get_fid(const char *path, __kernel_fsid_t *fsid,
}
}

+
+#ifndef FILEID_INVALID
+#define FILEID_INVALID 0xff
+#endif
+
struct fanotify_fid_t {
__kernel_fsid_t fsid;
struct file_handle handle;
@@ -424,4 +429,8 @@ struct fanotify_event_info_header *get_event_info(
((struct fanotify_event_info_error *) \
get_event_info((event), FAN_EVENT_INFO_TYPE_ERROR))

+#define get_event_info_fid(event) \
+ ((struct fanotify_event_info_fid *) \
+ get_event_info((event), FAN_EVENT_INFO_TYPE_FID))
+
#endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 31ad5cac0a0e..95c988821a34 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -40,6 +40,9 @@ int fd_notify;

#define MOUNT_PATH "test_mnt"

+/* These expected FIDs are common to multiple tests */
+static struct fanotify_fid_t null_fid;
+
static void trigger_fs_abort(void)
{
SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
@@ -50,6 +53,7 @@ static struct test_case {
char *name;
int error;
unsigned int error_count;
+ struct fanotify_fid_t *fid;
void (*trigger_error)(void);
} testcases[] = {
{
@@ -57,9 +61,43 @@ static struct test_case {
.trigger_error = &trigger_fs_abort,
.error_count = 1,
.error = ESHUTDOWN,
+ .fid = &null_fid,
},
};

+int check_error_event_info_fid(struct fanotify_event_info_fid *fid,
+ const struct test_case *ex)
+{
+ struct file_handle *fh = (struct file_handle *) &fid->handle;
+
+ if (memcmp(&fid->fsid, &ex->fid->fsid, sizeof(fid->fsid))) {
+ tst_res(TFAIL, "%s: Received bad FSID type (%x...!=%x...)",
+ ex->name, FSID_VAL_MEMBER(fid->fsid, 0),
+ FSID_VAL_MEMBER(ex->fid->fsid, 0));
+
+ return 1;
+ }
+ if (fh->handle_type != ex->fid->handle.handle_type) {
+ tst_res(TFAIL, "%s: Received bad file_handle type (%d!=%d)",
+ ex->name, fh->handle_type, ex->fid->handle.handle_type);
+ return 1;
+ }
+
+ if (fh->handle_bytes != ex->fid->handle.handle_bytes) {
+ tst_res(TFAIL, "%s: Received bad file_handle len (%d!=%d)",
+ ex->name, fh->handle_bytes, ex->fid->handle.handle_bytes);
+ return 1;
+ }
+
+ if (memcmp(fh->f_handle, ex->fid->handle.f_handle, fh->handle_bytes)) {
+ tst_res(TFAIL, "%s: Received wrong handle. "
+ "Expected (%x...) got (%x...) ", ex->name,
+ *(int*)ex->fid->handle.f_handle, *(int*)fh->f_handle);
+ return 1;
+ }
+ return 0;
+}
+
int check_error_event_info_error(struct fanotify_event_info_error *info_error,
const struct test_case *ex)
{
@@ -103,6 +141,7 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
struct fanotify_event_metadata *event =
(struct fanotify_event_metadata *) buf;
struct fanotify_event_info_error *info_error;
+ struct fanotify_event_info_fid *info_fid;
int fail = 0;

if (len < FAN_EVENT_METADATA_LEN) {
@@ -121,6 +160,14 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
fail++;
}

+ info_fid = get_event_info_fid(event);
+ if (info_fid)
+ fail += check_error_event_info_fid(info_fid, ex);
+ else {
+ tst_res(TFAIL, "FID record not found");
+ fail++;
+ }
+
if (!fail)
tst_res(TPASS, "Successfully received: %s", ex->name);
}
@@ -143,12 +190,25 @@ static void do_test(unsigned int i)
check_event(event_buf, read_len, tcase);
}

+static void init_null_fid(void)
+{
+ /* Use fanotify_save_fid to fill the fsid and overwrite the
+ * file_handler to create a null_fid
+ */
+ fanotify_save_fid(MOUNT_PATH, &null_fid);
+
+ null_fid.handle.handle_type = FILEID_INVALID;
+ null_fid.handle.handle_bytes = 0;
+}
+
static void setup(void)
{
REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_NOTIF|FAN_REPORT_FID,
FAN_MARK_FILESYSTEM,
FAN_FS_ERROR, ".");

+ init_null_fid();
+
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF|FAN_REPORT_FID,
O_RDONLY);
}
--
2.33.0

2021-10-29 21:20:06

[permalink] [raw]

Subject: [PATCH v3 7/9] syscalls/fanotify21: Create a corrupted file

Allocate a test directory and corrupt it with debugfs. The corruption
is done by writing an invalid inode mode. This file can be later
looked up to trigger a corruption error.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
testcases/kernel/syscalls/fanotify/fanotify21.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 43e606c27372..3e4ac2eb2e5b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -39,9 +39,12 @@ static char event_buf[BUF_SIZE];
int fd_notify;

#define MOUNT_PATH "test_mnt"
+#define BASE_DIR "internal_dir"
+#define BAD_DIR BASE_DIR"/bad_dir"

/* These expected FIDs are common to multiple tests */
static struct fanotify_fid_t null_fid;
+static struct fanotify_fid_t bad_file_fid;

static void trigger_fs_abort(void)
{
@@ -197,6 +200,18 @@ static void do_test(unsigned int i)
check_event(event_buf, read_len, tcase);
}

+static void pre_corrupt_fs(void)
+{
+ SAFE_MKDIR(MOUNT_PATH"/"BASE_DIR, 0777);
+ SAFE_MKDIR(MOUNT_PATH"/"BAD_DIR, 0777);
+
+ fanotify_save_fid(MOUNT_PATH"/"BAD_DIR, &bad_file_fid);
+
+ SAFE_UMOUNT(MOUNT_PATH);
+ do_debugfs_request(tst_device->dev, "sif " BAD_DIR " mode 0xff");
+ SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type, 0, NULL);
+}
+
static void init_null_fid(void)
{
/* Use fanotify_save_fid to fill the fsid and overwrite the
@@ -216,6 +231,8 @@ static void setup(void)

init_null_fid();

+ pre_corrupt_fs();
+
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF|FAN_REPORT_FID,
O_RDONLY);
}
--
2.33.0

2021-10-29 21:20:23

[permalink] [raw]

Subject: [PATCH v3 1/9] syscalls: fanotify: Add macro to require specific mark types

Like done for init flags and event types, and a macro to require a
specific mark type.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index a2be183385e4..c67db3117e29 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -373,4 +373,9 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
return rval;
}

+#define REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type) do { \
+ fanotify_init_flags_err_msg(#mark_type, __FILE__, __LINE__, tst_brk_, \
+ fanotify_mark_supported_by_kernel(mark_type)); \
+} while (0)
+
#endif /* __FANOTIFY_H__ */
--
2.33.0

2021-10-29 21:20:29

[permalink] [raw]

Subject: [PATCH v3 9/9] syscalls/fanotify21: Test capture of multiple errors

When multiple FS errors occur, only the first is stored. This testcase
validates this behavior by issuing two different errors and making sure
only the first is stored, while the second is simply accumulated in
error_count.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
.../kernel/syscalls/fanotify/fanotify21.c | 26 +++++++++++++++++++
1 file changed, 26 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index e463365dd69d..7f0154da5eeb 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -74,6 +74,18 @@ static void tcase2_trigger_lookup(void)
ret, BAD_DIR, errno, EUCLEAN);
}

+static void tcase3_trigger(void)
+{
+ trigger_fs_abort();
+ tcase2_trigger_lookup();
+}
+
+static void tcase4_trigger(void)
+{
+ tcase2_trigger_lookup();
+ trigger_fs_abort();
+}
+
static struct test_case {
char *name;
int error;
@@ -95,6 +107,20 @@ static struct test_case {
.error = EFSCORRUPTED,
.fid = &bad_file_fid,
},
+ {
+ .name = "Multiple error submission",
+ .trigger_error = &tcase3_trigger,
+ .error_count = 2,
+ .error = ESHUTDOWN,
+ .fid = &null_fid,
+ },
+ {
+ .name = "Multiple error submission 2",
+ .trigger_error = &tcase4_trigger,
+ .error_count = 2,
+ .error = EFSCORRUPTED,
+ .fid = &bad_file_fid,
+ }
};

int check_error_event_info_fid(struct fanotify_event_info_fid *fid,
--
2.33.0

2021-10-29 21:20:34

[permalink] [raw]

Subject: [PATCH v3 4/9] syscalls/fanotify21: Validate the generic error info

Implement some validation for the generic error info record emitted by
the kernel. The error number is fs-specific but, well, we only support
ext4 for now anyway.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

---
Changes since v2:
- check for error record type in autotools (Amir)
Changes since v1:
- Move defines to header file.
---
configure.ac | 3 +-
testcases/kernel/syscalls/fanotify/fanotify.h | 32 ++++++++++++++++
.../kernel/syscalls/fanotify/fanotify21.c | 37 ++++++++++++++++++-
3 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/configure.ac b/configure.ac
index 5bf3c52ec161..9efd5b33430f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -159,7 +159,8 @@ AC_CHECK_MEMBERS([struct utsname.domainname],,,[
AC_CHECK_TYPES([enum kcmp_type],,,[#include <linux/kcmp.h>])
AC_CHECK_TYPES([struct acct_v3],,,[#include <sys/acct.h>])
AC_CHECK_TYPES([struct af_alg_iv, struct sockaddr_alg],,,[# include <linux/if_alg.h>])
-AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header],,,[#include <sys/fanotify.h>])
+AC_CHECK_TYPES([struct fanotify_event_info_fid, struct fanotify_event_info_header,
+ struct fanotify_event_info_error],[],[],[#include <sys/fanotify.h>])
AC_CHECK_TYPES([struct file_dedupe_range],,,[#include <linux/fs.h>])

AC_CHECK_TYPES([struct file_handle],,,[
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 99b898554ede..4294799fe86d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -167,6 +167,9 @@ typedef struct {
#ifndef FAN_EVENT_INFO_TYPE_DFID
#define FAN_EVENT_INFO_TYPE_DFID 3
#endif
+#ifndef FAN_EVENT_INFO_TYPE_ERROR
+#define FAN_EVENT_INFO_TYPE_ERROR 5
+#endif

#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_HEADER
struct fanotify_event_info_header {
@@ -184,6 +187,14 @@ struct fanotify_event_info_fid {
};
#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID */

+#ifndef HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR
+struct fanotify_event_info_error {
+ struct fanotify_event_info_header hdr;
+ __s32 error;
+ __u32 error_count;
+};
+#endif /* HAVE_STRUCT_FANOTIFY_EVENT_INFO_ERROR */
+
/* NOTE: only for struct fanotify_event_info_fid */
#ifdef HAVE_STRUCT_FANOTIFY_EVENT_INFO_FID_FSID___VAL
# define FSID_VAL_MEMBER(fsid, i) (fsid.__val[i])
@@ -392,4 +403,25 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
} while (0)

+struct fanotify_event_info_header *get_event_info(
+ struct fanotify_event_metadata *event,
+ int info_type)
+{
+ struct fanotify_event_info_header *hdr = NULL;
+ char *start = (char *) event;
+ int off;
+
+ for (off = event->metadata_len; (off+sizeof(*hdr)) < event->event_len;
+ off += hdr->len) {
+ hdr = (struct fanotify_event_info_header *) &(start[off]);
+ if (hdr->info_type == info_type)
+ return hdr;
+ }
+ return NULL;
+}
+
+#define get_event_info_error(event) \
+ ((struct fanotify_event_info_error *) \
+ get_event_info((event), FAN_EVENT_INFO_TYPE_ERROR))
+
#endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 9ef687442b7c..31ad5cac0a0e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -48,14 +48,38 @@ static void trigger_fs_abort(void)

static struct test_case {
char *name;
+ int error;
+ unsigned int error_count;
void (*trigger_error)(void);
} testcases[] = {
{
.name = "Trigger abort",
.trigger_error = &trigger_fs_abort,
+ .error_count = 1,
+ .error = ESHUTDOWN,
},
};

+int check_error_event_info_error(struct fanotify_event_info_error *info_error,
+ const struct test_case *ex)
+{
+ int fail = 0;
+
+ if (info_error->error_count != ex->error_count) {
+ tst_res(TFAIL, "%s: Unexpected error_count (%d!=%d)",
+ ex->name, info_error->error_count, ex->error_count);
+ fail++;
+ }
+
+ if (info_error->error != ex->error) {
+ tst_res(TFAIL, "%s: Unexpected error code value (%d!=%d)",
+ ex->name, info_error->error, ex->error);
+ fail++;
+ }
+
+ return fail;
+}
+
int check_error_event_metadata(struct fanotify_event_metadata *event)
{
int fail = 0;
@@ -78,6 +102,8 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
{
struct fanotify_event_metadata *event =
(struct fanotify_event_metadata *) buf;
+ struct fanotify_event_info_error *info_error;
+ int fail = 0;

if (len < FAN_EVENT_METADATA_LEN) {
tst_res(TFAIL, "No event metadata found");
@@ -87,7 +113,16 @@ void check_event(char *buf, size_t len, const struct test_case *ex)
if (check_error_event_metadata(event))
return;

- tst_res(TPASS, "Successfully received: %s", ex->name);
+ info_error = get_event_info_error(event);
+ if (info_error)
+ fail += check_error_event_info_error(info_error, ex);
+ else {
+ tst_res(TFAIL, "Generic error record not found");
+ fail++;
+ }
+
+ if (!fail)
+ tst_res(TPASS, "Successfully received: %s", ex->name);
}

static void do_test(unsigned int i)
--
2.33.0

2021-10-29 21:20:34

[permalink] [raw]

Subject: [PATCH v3 6/9] syscalls/fanotify21: Support submission of debugfs commands

In order to test FAN_FS_ERROR, we want to corrupt the filesystem. The
easiest way to do it is by using debugfs. Add a small helper to issue
debugfs requests. Since most likely this will be the only testcase to
need this, don't bother making it a proper helper for now.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
changes since v1:
- Add .needs_cmds to require debugfs
---
testcases/kernel/syscalls/fanotify/fanotify21.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 95c988821a34..43e606c27372 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -49,6 +49,13 @@ static void trigger_fs_abort(void)
MS_REMOUNT|MS_RDONLY, "abort");
}

+static void do_debugfs_request(const char *dev, char *request)
+{
+ const char *cmd[] = {"debugfs", "-w", dev, "-R", request, NULL};
+
+ SAFE_CMD(cmd, NULL, NULL);
+}
+
static struct test_case {
char *name;
int error;
@@ -228,7 +235,11 @@ static struct tst_test test = {
.mntpoint = MOUNT_PATH,
.all_filesystems = 0,
.needs_root = 1,
- .dev_fs_type = "ext4"
+ .dev_fs_type = "ext4",
+ .needs_cmds = (const char *[]) {
+ "debugfs",
+ NULL
+ }
};

#else
--
2.33.0

2021-10-29 21:21:34

[permalink] [raw]

Subject: [PATCH v3 8/9] syscalls/fanotify21: Test file event with broken inode

This test corrupts an inode entry with an invalid mode through debugfs
and then tries to access it. This should result in a ext4 error, which
we monitor through the fanotify group.

Reviewed-by: Amir Goldstein <[email protected]>
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
.../kernel/syscalls/fanotify/fanotify21.c | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 3e4ac2eb2e5b..e463365dd69d 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -34,6 +34,10 @@
#ifdef HAVE_SYS_FANOTIFY_H
#include "fanotify.h"

+#ifndef EFSCORRUPTED
+#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#endif
+
#define BUF_SIZE 256
static char event_buf[BUF_SIZE];
int fd_notify;
@@ -59,6 +63,17 @@ static void do_debugfs_request(const char *dev, char *request)
SAFE_CMD(cmd, NULL, NULL);
}

+static void tcase2_trigger_lookup(void)
+{
+ int ret;
+
+ /* SAFE_OPEN cannot be used here because we expect it to fail. */
+ ret = open(MOUNT_PATH"/"BAD_DIR, O_RDONLY, 0);
+ if (ret != -1 && errno != EUCLEAN)
+ tst_res(TFAIL, "Unexpected lookup result(%d) of %s (%d!=%d)",
+ ret, BAD_DIR, errno, EUCLEAN);
+}
+
static struct test_case {
char *name;
int error;
@@ -73,6 +88,13 @@ static struct test_case {
.error = ESHUTDOWN,
.fid = &null_fid,
},
+ {
+ .name = "Lookup of inode with invalid mode",
+ .trigger_error = &tcase2_trigger_lookup,
+ .error_count = 1,
+ .error = EFSCORRUPTED,
+ .fid = &bad_file_fid,
+ },
};

int check_error_event_info_fid(struct fanotify_event_info_fid *fid,
--
2.33.0

2021-10-30 06:18:16

[permalink] [raw]

Subject: Re: [PATCH v3 2/9] syscalls: fanotify: Add macro to require specific events

On Sat, Oct 30, 2021 at 12:18 AM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Add a helper for tests to fail if an event is not available in the
> kernel. Since some events only work with REPORT_FID or a specific
> class, update the verifier to allow those to be specified.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>

Reviewed-by: Amir Goldstein <[email protected]>

> ---
> Changes since v1:
> - Use SAFE_FANOTIFY_INIT instead of open coding. (Amir)
> - Use FAN_CLASS_NOTIF for fanotify12 testcase. (Amir)
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 17 ++++++++++++++---
> testcases/kernel/syscalls/fanotify/fanotify03.c | 4 ++--
> testcases/kernel/syscalls/fanotify/fanotify10.c | 3 ++-
> testcases/kernel/syscalls/fanotify/fanotify12.c | 3 ++-
> 4 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index c67db3117e29..820073709571 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -266,14 +266,16 @@ static inline void require_fanotify_access_permissions_supported_by_kernel(void)
> SAFE_CLOSE(fd);
> }
>
> -static inline int fanotify_events_supported_by_kernel(uint64_t mask)
> +static inline int fanotify_events_supported_by_kernel(uint64_t mask,
> + unsigned int init_flags,
> + unsigned int mark_flags)
> {
> int fd;
> int rval = 0;
>
> - fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> + fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);
>
> - if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
> + if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> if (errno == EINVAL) {
> rval = -1;
> } else {
> @@ -378,4 +380,13 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> fanotify_mark_supported_by_kernel(mark_type)); \
> } while (0)
>
> +#define REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(init_flags, mark_type, mask, fname) do { \
> + if (mark_type) \
> + REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type); \
> + if (init_flags) \
> + REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(init_flags, fname); \
> + fanotify_init_flags_err_msg(#mask, __FILE__, __LINE__, tst_brk_, \
> + fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
> +} while (0)
> +
> #endif /* __FANOTIFY_H__ */
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index 26d17e64d1f5..2081f0bd1b57 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> @@ -323,8 +323,8 @@ static void setup(void)
> require_fanotify_access_permissions_supported_by_kernel();
>
> filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
> -
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM,
> + FAN_CLASS_CONTENT, 0);
> sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
> SAFE_FILE_PRINTF(fname, "1");
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index 92e4d3ff3054..0fa9d1f4f7e4 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -509,7 +509,8 @@ cleanup:
>
> static void setup(void)
> {
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
> + FAN_CLASS_CONTENT, 0);
> filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
> MOUNT_PATH);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
> index 76f1aca77615..c77dbfd8c23d 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify12.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
> @@ -222,7 +222,8 @@ cleanup:
>
> static void do_setup(void)
> {
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
> + FAN_CLASS_NOTIF, 0);
>
> sprintf(fname, "fname_%d", getpid());
> SAFE_FILE_PRINTF(fname, "1");
> --
> 2.33.0
>

2021-10-30 06:21:27

[permalink] [raw]

Subject: Re: [PATCH v3 3/9] syscalls/fanotify21: Introduce FAN_FS_ERROR test

On Sat, Oct 30, 2021 at 12:18 AM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> fanotify21 is a new test validating the FAN_FS_ERROR file system error
> event. This adds some basic structure for the next patches and a single
> test of error reporting during filesystem abort
>
> The strategy for error reporting testing in fanotify21 goes like this:
>
> - Generate a broken filesystem
> - Start FAN_FS_ERROR monitoring group
> - Make the file system notice the error through ordinary operations
> - Observe the event generated
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>

Reviewed-by: Amir Goldstein <[email protected]>

> ---
> Changes since v1:
> - Move defines to header file.
> - Move fanotify_mark(2) to do_test (Amir)
> - Merge abort test here
> ---
> testcases/kernel/syscalls/fanotify/.gitignore | 1 +
> testcases/kernel/syscalls/fanotify/fanotify.h | 3 +
> .../kernel/syscalls/fanotify/fanotify21.c | 141 ++++++++++++++++++
> 3 files changed, 145 insertions(+)
> create mode 100644 testcases/kernel/syscalls/fanotify/fanotify21.c
>
> diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
> index 9554b16b196e..79ad184d578b 100644
> --- a/testcases/kernel/syscalls/fanotify/.gitignore
> +++ b/testcases/kernel/syscalls/fanotify/.gitignore
> @@ -17,4 +17,5 @@
> /fanotify17
> /fanotify18
> /fanotify19
> +/fanotify21
> /fanotify_child
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 820073709571..99b898554ede 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -124,6 +124,9 @@ static inline int safe_fanotify_mark(const char *file, const int lineno,
> #ifndef FAN_OPEN_EXEC_PERM
> #define FAN_OPEN_EXEC_PERM 0x00040000
> #endif
> +#ifndef FAN_FS_ERROR
> +#define FAN_FS_ERROR 0x00008000
> +#endif
>
> /* Flags required for unprivileged user group */
> #define FANOTIFY_REQUIRED_USER_INIT_FLAGS (FAN_REPORT_FID)
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
> new file mode 100644
> index 000000000000..9ef687442b7c
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021 Collabora Ltd.
> + *
> + * Author: Gabriel Krisman Bertazi <[email protected]>
> + * Based on previous work by Amir Goldstein <[email protected]>
> + */
> +
> +/*\
> + * [Description]
> + * Check fanotify FAN_ERROR_FS events triggered by intentionally
> + * corrupted filesystems:
> + *
> + * - Generate a broken filesystem
> + * - Start FAN_FS_ERROR monitoring group
> + * - Make the file system notice the error through ordinary operations
> + * - Observe the event generated
> + */
> +
> +#define _GNU_SOURCE
> +#include "config.h"
> +
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include <errno.h>
> +#include <string.h>
> +#include <sys/mount.h>
> +#include <sys/syscall.h>
> +#include "tst_test.h"
> +#include <sys/fanotify.h>
> +#include <sys/types.h>
> +#include <fcntl.h>
> +
> +#ifdef HAVE_SYS_FANOTIFY_H
> +#include "fanotify.h"
> +
> +#define BUF_SIZE 256
> +static char event_buf[BUF_SIZE];
> +int fd_notify;
> +
> +#define MOUNT_PATH "test_mnt"
> +
> +static void trigger_fs_abort(void)
> +{
> + SAFE_MOUNT(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
> + MS_REMOUNT|MS_RDONLY, "abort");
> +}
> +
> +static struct test_case {
> + char *name;
> + void (*trigger_error)(void);
> +} testcases[] = {
> + {
> + .name = "Trigger abort",
> + .trigger_error = &trigger_fs_abort,
> + },
> +};
> +
> +int check_error_event_metadata(struct fanotify_event_metadata *event)
> +{
> + int fail = 0;
> +
> + if (event->mask != FAN_FS_ERROR) {
> + fail++;
> + tst_res(TFAIL, "got unexpected event %llx",
> + (unsigned long long)event->mask);
> + }
> +
> + if (event->fd != FAN_NOFD) {
> + fail++;
> + tst_res(TFAIL, "Weird FAN_FD %llx",
> + (unsigned long long)event->mask);
> + }
> + return fail;
> +}
> +
> +void check_event(char *buf, size_t len, const struct test_case *ex)
> +{
> + struct fanotify_event_metadata *event =
> + (struct fanotify_event_metadata *) buf;
> +
> + if (len < FAN_EVENT_METADATA_LEN) {
> + tst_res(TFAIL, "No event metadata found");
> + return;
> + }
> +
> + if (check_error_event_metadata(event))
> + return;
> +
> + tst_res(TPASS, "Successfully received: %s", ex->name);
> +}
> +
> +static void do_test(unsigned int i)
> +{
> + const struct test_case *tcase = &testcases[i];
> + size_t read_len;
> +
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD|FAN_MARK_FILESYSTEM,
> + FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
> +
> + tcase->trigger_error();
> +
> + read_len = SAFE_READ(0, fd_notify, event_buf, BUF_SIZE);
> +
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE|FAN_MARK_FILESYSTEM,
> + FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
> +
> + check_event(event_buf, read_len, tcase);
> +}
> +
> +static void setup(void)
> +{
> + REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(FAN_CLASS_NOTIF|FAN_REPORT_FID,
> + FAN_MARK_FILESYSTEM,
> + FAN_FS_ERROR, ".");
> +
> + fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF|FAN_REPORT_FID,
> + O_RDONLY);
> +}
> +
> +static void cleanup(void)
> +{
> + if (fd_notify > 0)
> + SAFE_CLOSE(fd_notify);
> +}
> +
> +static struct tst_test test = {
> + .test = do_test,
> + .tcnt = ARRAY_SIZE(testcases),
> + .setup = setup,
> + .cleanup = cleanup,
> + .mount_device = 1,
> + .mntpoint = MOUNT_PATH,
> + .all_filesystems = 0,

That's probably redundant and the default value anyway.
If you want to stress out that this test cannot be run on other filesystems
maybe add a comment why that is above dev_fs_type.

> + .needs_root = 1,
> + .dev_fs_type = "ext4"
> +};
> +

Thanks,
Amir.

2021-10-30 06:22:21

[permalink] [raw]

Subject: Re: [PATCH v3 0/9] Test the new fanotify FAN_FS_ERROR event

On Sat, Oct 30, 2021 at 12:17 AM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Hi,
>
> Now that FAN_FS_ERROR is close to being merged, I'm sending a new
> version of the LTP tests. This is the v3 of this patchset, and it
> applies the feedback of the previous version, in particular, it solves
> the issue Amir pointed out, that ltp won't gracefully handle a test with
> tcnt==0. To solve that, I merged the patch that set up the environment
> with a simple test, that only triggers a fs abort and watches the
> event.
>
> I'm also renaming the testcase from fanotify20 to fanotify21, to leave
> room for the pidfs test that is also in the baking by Matthew Bobrowski.

Only Matthew posted two tests... anyway I don't think merge is going to be
a big problem. I am hoping that PIDFD tests could be merged soonish and
I recommended Matthew to base his tests over your first two macro prep
patches.

Thanks,
Amir.

2021-11-02 11:28:24

by Matt Bobrowski

[permalink] [raw]

Subject: Re: [PATCH v3 1/9] syscalls: fanotify: Add macro to require specific mark types

On Fri, Oct 29, 2021 at 06:17:24PM -0300, Gabriel Krisman Bertazi wrote:
> Like done for init flags and event types, and a macro to require a
> specific mark type.
>
> Reviewed-by: Amir Goldstein <[email protected]>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index a2be183385e4..c67db3117e29 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -373,4 +373,9 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> return rval;
> }
>
> +#define REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type) do { \
> + fanotify_init_flags_err_msg(#mark_type, __FILE__, __LINE__, tst_brk_, \
> + fanotify_mark_supported_by_kernel(mark_type)); \
> +} while (0)
> +
> #endif /* __FANOTIFY_H__ */

A nit, but I'm of the opinion that s/_ON_/_BY_ within the macro name. Otherwise,
this looks OK to me.

Reviewed-by: Matthew Bobrowski <[email protected]>

/M

2021-11-02 11:59:49

by Matt Bobrowski

[permalink] [raw]

Subject: Re: [PATCH v3 2/9] syscalls: fanotify: Add macro to require specific events

On Fri, Oct 29, 2021 at 06:17:25PM -0300, Gabriel Krisman Bertazi wrote:
> Add a helper for tests to fail if an event is not available in the
> kernel. Since some events only work with REPORT_FID or a specific
> class, update the verifier to allow those to be specified.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

Made a single comment, otherwise this looks OK to me.

Reviewed-by: Matthew Bobrowski <[email protected]>

> ---
> Changes since v1:
> - Use SAFE_FANOTIFY_INIT instead of open coding. (Amir)
> - Use FAN_CLASS_NOTIF for fanotify12 testcase. (Amir)
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 17 ++++++++++++++---
> testcases/kernel/syscalls/fanotify/fanotify03.c | 4 ++--
> testcases/kernel/syscalls/fanotify/fanotify10.c | 3 ++-
> testcases/kernel/syscalls/fanotify/fanotify12.c | 3 ++-
> 4 files changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index c67db3117e29..820073709571 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -266,14 +266,16 @@ static inline void require_fanotify_access_permissions_supported_by_kernel(void)
> SAFE_CLOSE(fd);
> }
>
> -static inline int fanotify_events_supported_by_kernel(uint64_t mask)
> +static inline int fanotify_events_supported_by_kernel(uint64_t mask,
> + unsigned int init_flags,
> + unsigned int mark_flags)
> {
> int fd;
> int rval = 0;
>
> - fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> + fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);
>
> - if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
> + if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> if (errno == EINVAL) {
> rval = -1;
> } else {
> @@ -378,4 +380,13 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> fanotify_mark_supported_by_kernel(mark_type)); \
> } while (0)
>
> +#define REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(init_flags, mark_type, mask, fname) do { \
> + if (mark_type) \
> + REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type); \
> + if (init_flags) \
> + REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(init_flags, fname); \
> + fanotify_init_flags_err_msg(#mask, __FILE__, __LINE__, tst_brk_, \
> + fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
> +} while (0)
> +
> #endif /* __FANOTIFY_H__ */
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index 26d17e64d1f5..2081f0bd1b57 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> @@ -323,8 +323,8 @@ static void setup(void)
> require_fanotify_access_permissions_supported_by_kernel();
>
> filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
> -
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM,
> + FAN_CLASS_CONTENT, 0);
> sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
> SAFE_FILE_PRINTF(fname, "1");
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index 92e4d3ff3054..0fa9d1f4f7e4 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -509,7 +509,8 @@ cleanup:
>
> static void setup(void)
> {
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
> + FAN_CLASS_CONTENT, 0);

I'm wondering whether this is the best combination of mask and
init_flags to use in this particular case? Maybe not to confuse future
readers, using FAN_CLASS_NOTIF explicitly here would be better, WDYT?
It doesn't make a difference, but it's something that caught my eye
while parsing this patch.

> filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
> MOUNT_PATH);
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
> index 76f1aca77615..c77dbfd8c23d 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify12.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
> @@ -222,7 +222,8 @@ cleanup:
>
> static void do_setup(void)
> {
> - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
> + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
> + FAN_CLASS_NOTIF, 0);
>
> sprintf(fname, "fname_%d", getpid());
> SAFE_FILE_PRINTF(fname, "1");
> --
> 2.33.0

/M

2021-11-02 12:15:04

[permalink] [raw]

Subject: Re: [PATCH v3 1/9] syscalls: fanotify: Add macro to require specific mark types

On Tue, Nov 2, 2021 at 1:27 PM Matthew Bobrowski <[email protected]> wrote:
>
> On Fri, Oct 29, 2021 at 06:17:24PM -0300, Gabriel Krisman Bertazi wrote:
> > Like done for init flags and event types, and a macro to require a
> > specific mark type.
> >
> > Reviewed-by: Amir Goldstein <[email protected]>
> > Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> > ---
> > testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > index a2be183385e4..c67db3117e29 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> > @@ -373,4 +373,9 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> > return rval;
> > }
> >
> > +#define REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type) do { \
> > + fanotify_init_flags_err_msg(#mark_type, __FILE__, __LINE__, tst_brk_, \
> > + fanotify_mark_supported_by_kernel(mark_type)); \
> > +} while (0)
> > +
> > #endif /* __FANOTIFY_H__ */
>
> A nit, but I'm of the opinion that s/_ON_/_BY_ within the macro name. Otherwise,
> this looks OK to me.

Agreed. You can change that while cherry-picking to your branch ;-)

Thanks,
Amir.

2021-11-02 12:15:05

[permalink] [raw]

Subject: Re: [PATCH v3 2/9] syscalls: fanotify: Add macro to require specific events

On Tue, Nov 2, 2021 at 1:59 PM Matthew Bobrowski <[email protected]> wrote:
>
> On Fri, Oct 29, 2021 at 06:17:25PM -0300, Gabriel Krisman Bertazi wrote:
> > Add a helper for tests to fail if an event is not available in the
> > kernel. Since some events only work with REPORT_FID or a specific
> > class, update the verifier to allow those to be specified.
> >
> > Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>
> Made a single comment, otherwise this looks OK to me.
>
> Reviewed-by: Matthew Bobrowski <[email protected]>
>
> > ---
> > Changes since v1:
> > - Use SAFE_FANOTIFY_INIT instead of open coding. (Amir)
> > - Use FAN_CLASS_NOTIF for fanotify12 testcase. (Amir)
> > ---
> > testcases/kernel/syscalls/fanotify/fanotify.h | 17 ++++++++++++++---
> > testcases/kernel/syscalls/fanotify/fanotify03.c | 4 ++--
> > testcases/kernel/syscalls/fanotify/fanotify10.c | 3 ++-
> > testcases/kernel/syscalls/fanotify/fanotify12.c | 3 ++-
> > 4 files changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > index c67db3117e29..820073709571 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> > @@ -266,14 +266,16 @@ static inline void require_fanotify_access_permissions_supported_by_kernel(void)
> > SAFE_CLOSE(fd);
> > }
> >
> > -static inline int fanotify_events_supported_by_kernel(uint64_t mask)
> > +static inline int fanotify_events_supported_by_kernel(uint64_t mask,
> > + unsigned int init_flags,
> > + unsigned int mark_flags)
> > {
> > int fd;
> > int rval = 0;
> >
> > - fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> > + fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);
> >
> > - if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
> > + if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> > if (errno == EINVAL) {
> > rval = -1;
> > } else {
> > @@ -378,4 +380,13 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> > fanotify_mark_supported_by_kernel(mark_type)); \
> > } while (0)
> >
> > +#define REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS(init_flags, mark_type, mask, fname) do { \
> > + if (mark_type) \
> > + REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type); \
> > + if (init_flags) \
> > + REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(init_flags, fname); \
> > + fanotify_init_flags_err_msg(#mask, __FILE__, __LINE__, tst_brk_, \
> > + fanotify_events_supported_by_kernel(mask, init_flags, mark_type)); \
> > +} while (0)
> > +
> > #endif /* __FANOTIFY_H__ */
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > index 26d17e64d1f5..2081f0bd1b57 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> > @@ -323,8 +323,8 @@ static void setup(void)
> > require_fanotify_access_permissions_supported_by_kernel();
> >
> > filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> > - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
> > -
> > + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC_PERM,
> > + FAN_CLASS_CONTENT, 0);
> > sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
> > SAFE_FILE_PRINTF(fname, "1");
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > index 92e4d3ff3054..0fa9d1f4f7e4 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > @@ -509,7 +509,8 @@ cleanup:
> >
> > static void setup(void)
> > {
> > - exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC);
> > + exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
> > + FAN_CLASS_CONTENT, 0);
>
> I'm wondering whether this is the best combination of mask and
> init_flags to use in this particular case? Maybe not to confuse future
> readers, using FAN_CLASS_NOTIF explicitly here would be better, WDYT?
> It doesn't make a difference, but it's something that caught my eye
> while parsing this patch.
>

Wow.
I think you are right in that this test does not use the combination
FAN_OPEN_EXEC with FAN_CLASS_CONTENT group, but it is quite hard
figure this out.

It will also be quite hard to figure out if this ever changes if ever new
test cases are added, so it will be hard to catch a change like that in review.
Given all that I would not change the requirement.

Thanks,
Amir.

2021-11-03 16:14:16

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 1/9] syscalls: fanotify: Add macro to require specific mark types

Hi all,

> On Tue, Nov 2, 2021 at 1:27 PM Matthew Bobrowski <[email protected]> wrote:

> > On Fri, Oct 29, 2021 at 06:17:24PM -0300, Gabriel Krisman Bertazi wrote:
> > > Like done for init flags and event types, and a macro to require a
> > > specific mark type.

> > > Reviewed-by: Amir Goldstein <[email protected]>
> > > Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> > > ---
> > > testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++++
> > > 1 file changed, 5 insertions(+)

> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > > index a2be183385e4..c67db3117e29 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> > > @@ -373,4 +373,9 @@ static inline int fanotify_mark_supported_by_kernel(uint64_t flag)
> > > return rval;
> > > }

> > > +#define REQUIRE_MARK_TYPE_SUPPORTED_ON_KERNEL(mark_type) do { \
> > > + fanotify_init_flags_err_msg(#mark_type, __FILE__, __LINE__, tst_brk_, \
> > > + fanotify_mark_supported_by_kernel(mark_type)); \
> > > +} while (0)
> > > +
> > > #endif /* __FANOTIFY_H__ */

> > A nit, but I'm of the opinion that s/_ON_/_BY_ within the macro name. Otherwise,
> > this looks OK to me.

> Agreed. You can change that while cherry-picking to your branch ;-)

+1. And yes, I'll change it while applying it into LTP (no need to repost).

Reviewed-by: Petr Vorel <[email protected]>

Kind regards,
Petr

> Thanks,
> Amir.

2021-11-05 09:10:51

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 3/9] syscalls/fanotify21: Introduce FAN_FS_ERROR test

Hi all,

...
> > +static struct tst_test test = {
> > + .test = do_test,
> > + .tcnt = ARRAY_SIZE(testcases),
> > + .setup = setup,
> > + .cleanup = cleanup,
> > + .mount_device = 1,
> > + .mntpoint = MOUNT_PATH,
> > + .all_filesystems = 0,

> That's probably redundant and the default value anyway.
> If you want to stress out that this test cannot be run on other filesystems
> maybe add a comment why that is above dev_fs_type.

Yes, good catch. Thanks!

Kind regards,
Petr

2021-11-05 09:50:39

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 5/9] syscalls/fanotify21: Validate incoming FID in FAN_FS_ERROR

Hi all,

Reviewed-by: Petr Vorel <[email protected]>

...
> +int check_error_event_info_fid(struct fanotify_event_info_fid *fid,
> + const struct test_case *ex)
> +{
> + struct file_handle *fh = (struct file_handle *) &fid->handle;
> +
> + if (memcmp(&fid->fsid, &ex->fid->fsid, sizeof(fid->fsid))) {
> + tst_res(TFAIL, "%s: Received bad FSID type (%x...!=%x...)",
> + ex->name, FSID_VAL_MEMBER(fid->fsid, 0),
> + FSID_VAL_MEMBER(ex->fid->fsid, 0));
Correct way is (I'll fix it before pushing this PR):

if (memcmp(&fid->fsid, &ex->fid->fsid, sizeof(fid->fsid))) {
tst_res(TFAIL, "%s: Received bad FSID type (%x...!=%x...)",
ex->name, FSID_VAL_MEMBER(fid->fsid, 0),
ex->fid->fsid.val[0]);

Using FSID_VAL_MEMBER() is invalid, it broke compilation for musl.
FSID_VAL_MEMBER was created to fix musl compilations, but it should be used only
for struct fanotify_event_info_fid, using it for struct event_t (which has also
__kernel_fsid_t fsid) is invalid and causes this error:

In file included from fanotify21.c:35:
fanotify21.c: In function 'check_error_event_info_fid':
fanotify.h:200:41: error: 'lapi_fsid_t' has no member named '__val'; did you mean 'val'?
200 | # define FSID_VAL_MEMBER(fsid, i) (fsid.__val[i])
| ^~~~~
../../../../include/tst_test.h:58:54: note: in expansion of macro 'FSID_VAL_MEMBER'
58 | tst_res_(__FILE__, __LINE__, (ttype), (arg_fmt), ##__VA_ARGS__);\
| ^~~~~~~~~~~
fanotify21.c:132:3: note: in expansion of macro 'tst_res'
132 | tst_res(TFAIL, "%s: Received bad FSID type (%x...!=%x...)",
| ^~~~~~~
make: *** [../../../../include/mk/rules.mk:37: fanotify21] Error 1

Sorry for confusion, not sure how to improve FSID_VAL_MEMBER() to avoid these
errors.

See f37704d6c ("fanotify: Fix FSID_VAL_MEMBER() usage")

Kind regards,
Petr

2021-11-05 10:10:56

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 0/9] Test the new fanotify FAN_FS_ERROR event

Hi Gabriel, all,

> Hi,

> Now that FAN_FS_ERROR is close to being merged, I'm sending a new
> version of the LTP tests. This is the v3 of this patchset, and it
> applies the feedback of the previous version, in particular, it solves
> the issue Amir pointed out, that ltp won't gracefully handle a test with
> tcnt==0. To solve that, I merged the patch that set up the environment
> with a simple test, that only triggers a fs abort and watches the
> event.

> I'm also renaming the testcase from fanotify20 to fanotify21, to leave
> room for the pidfs test that is also in the baking by Matthew Bobrowski.

> One important detail is that, for the tests to succeed, there is a
> dependency on an ext4 fix I sent a few days ago:

> https://lore.kernel.org/linux-ext4/[email protected]/T/#u
It has been merged into Theodore Ts'o ext4 tree into dev branch as c1e2e0350ce3
("ext4: Fix error code saved on super block during file system abort")

We should probably add it as .tags (see fanotify06.c).

Also it'd be nice just to mention relevant commits which added FAN_FS_ERROR in
fanotify21.c (probably "fanotify: Allow users to request FAN_FS_ERROR events" ?)
+ kernel version it added it -suppose 5.16 (although it can be backported; and
these commits should not go to .tags, as we track there only
fixes not new features). I can add it myself (no need to repost).

Kind regards,
Petr

2021-11-05 13:04:10

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 0/9] Test the new fanotify FAN_FS_ERROR event

On Fri, Nov 5, 2021 at 12:10 PM Petr Vorel <[email protected]> wrote:
>
> Hi Gabriel, all,
>
> > Hi,
>
> > Now that FAN_FS_ERROR is close to being merged, I'm sending a new
> > version of the LTP tests. This is the v3 of this patchset, and it
> > applies the feedback of the previous version, in particular, it solves
> > the issue Amir pointed out, that ltp won't gracefully handle a test with
> > tcnt==0. To solve that, I merged the patch that set up the environment
> > with a simple test, that only triggers a fs abort and watches the
> > event.
>
> > I'm also renaming the testcase from fanotify20 to fanotify21, to leave
> > room for the pidfs test that is also in the baking by Matthew Bobrowski.
>
> > One important detail is that, for the tests to succeed, there is a
> > dependency on an ext4 fix I sent a few days ago:
>
> > https://lore.kernel.org/linux-ext4/[email protected]/T/#u
> It has been merged into Theodore Ts'o ext4 tree into dev branch as c1e2e0350ce3
> ("ext4: Fix error code saved on super block during file system abort")
>
> We should probably add it as .tags (see fanotify06.c).

No point in doing that.
There will be no kernel release which meets the test requirements
and has this bug.

Thanks,
Amir.

2021-11-05 13:12:48

[permalink] [raw]

Subject: Re: [LTP] [PATCH v3 0/9] Test the new fanotify FAN_FS_ERROR event

> On Fri, Nov 5, 2021 at 12:10 PM Petr Vorel <[email protected]> wrote:

> > Hi Gabriel, all,

> > > Hi,

> > > Now that FAN_FS_ERROR is close to being merged, I'm sending a new
> > > version of the LTP tests. This is the v3 of this patchset, and it
> > > applies the feedback of the previous version, in particular, it solves
> > > the issue Amir pointed out, that ltp won't gracefully handle a test with
> > > tcnt==0. To solve that, I merged the patch that set up the environment
> > > with a simple test, that only triggers a fs abort and watches the
> > > event.

> > > I'm also renaming the testcase from fanotify20 to fanotify21, to leave
> > > room for the pidfs test that is also in the baking by Matthew Bobrowski.

> > > One important detail is that, for the tests to succeed, there is a
> > > dependency on an ext4 fix I sent a few days ago:

> > > https://lore.kernel.org/linux-ext4/[email protected]/T/#u
> > It has been merged into Theodore Ts'o ext4 tree into dev branch as c1e2e0350ce3
> > ("ext4: Fix error code saved on super block during file system abort")

> > We should probably add it as .tags (see fanotify06.c).

> No point in doing that.
> There will be no kernel release which meets the test requirements
> and has this bug.
Yes, that's correct for stable kernels, enterprise kernels can backport
FAN_FS_ERROR feature, while miss this fix (it does not mention Fixes).

Kind regards,
Petr

> Thanks,
> Amir.

2021-11-15 21:38:23