2021-10-27 05:59:48

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 00/10] Test the new fanotify FAN_FS_ERROR event

Hi,

Now that FAN_FS_ERROR is close to being merged, I'm sending out a new
version of the LTP tests. This version only applies the previous
feedback and updates the interface to correspond to the changes
requested on the kernel patches.

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

https://lore.kernel.org/linux-ext4/[email protected]/T/#u

---

Original cover letter:

FAN_FS_ERROR is a new (still unmerged) fanotify event to monitor
fileystem errors. This patchset introduces a new LTP test for this
feature.

Testing file system errors is slightly tricky, in particular because
they are mostly file system dependent. Since there are only patches for
ext4, I choose to make the test around it, since there wouldn't be much
to do with other file systems. The second challenge is how we cause the
file system errors, since there is no error injection for ext4 in Linux.
In this series, this is done by corrupting specific data in the
test device with the help of debugfs.

The FAN_FS_ERROR feature is flying around linux-ext4 and fsdevel, and
the latest version is available on the branch below:

https://gitlab.collabora.com/krisman/linux -b fanotify-notifications-v9

A proper manpage description is also available on the respective mailing
list, or in the branch below:

https://gitlab.collabora.com/krisman/man-pages.git -b fan-fs-error

Please, let me know your thoughts.

Gabriel Krisman Bertazi (10):
syscalls: fanotify: Add macro to require specific mark types
syscalls: fanotify: Add macro to require specific events
syscalls/fanotify20: Introduce helpers for FAN_FS_ERROR test
syscalls/fanotify20: Validate the generic error info
syscalls/fanotify20: Validate incoming FID in FAN_FS_ERROR
syscalls/fanotify20: Support submission of debugfs commands
syscalls/fanotify20: Create a corrupted file
syscalls/fanotify20: Test event after filesystem abort
syscalls/fanotify20: Test file event with broken inode
syscalls/fanotify20: Test capture of multiple errors

testcases/kernel/syscalls/fanotify/.gitignore | 1 +
testcases/kernel/syscalls/fanotify/fanotify.h | 72 +++-
.../kernel/syscalls/fanotify/fanotify03.c | 4 +-
.../kernel/syscalls/fanotify/fanotify10.c | 3 +-
.../kernel/syscalls/fanotify/fanotify12.c | 3 +-
.../kernel/syscalls/fanotify/fanotify20.c | 313 ++++++++++++++++++
6 files changed, 389 insertions(+), 7 deletions(-)
create mode 100644 testcases/kernel/syscalls/fanotify/fanotify20.c

--
2.33.0


2021-10-27 06:00:40

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 01/10] 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.

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-27 06:00:58

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 03/10] syscalls/fanotify20: Introduce helpers for FAN_FS_ERROR test

fanotify20 is a new test validating the FAN_FS_ERROR file system error
event. This adds some basic structure for the next patches.

The strategy for error reporting testing in fanotify20 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.
---
testcases/kernel/syscalls/fanotify/.gitignore | 1 +
testcases/kernel/syscalls/fanotify/fanotify.h | 3 +
.../kernel/syscalls/fanotify/fanotify20.c | 128 ++++++++++++++++++
3 files changed, 132 insertions(+)
create mode 100644 testcases/kernel/syscalls/fanotify/fanotify20.c

diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
index 9554b16b196e..c99e6fff76d6 100644
--- a/testcases/kernel/syscalls/fanotify/.gitignore
+++ b/testcases/kernel/syscalls/fanotify/.gitignore
@@ -17,4 +17,5 @@
/fanotify17
/fanotify18
/fanotify19
+/fanotify20
/fanotify_child
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index b2b56466d028..8828b53532a2 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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
new file mode 100644
index 000000000000..7a522aad4386
--- /dev/null
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -0,0 +1,128 @@
+// 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 struct test_case {
+ char *name;
+ void (*trigger_error)(void);
+} testcases[] = {
+};
+
+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;
+
+ tcase->trigger_error();
+
+ read_len = SAFE_READ(0, fd_notify, event_buf, BUF_SIZE);
+
+ 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);
+
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD|FAN_MARK_FILESYSTEM,
+ FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);
+}
+
+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-27 06:01:08

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 09/10] syscalls/fanotify20: 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.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 5c5ee3c6fb74..7bcddcaa98cb 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -38,6 +38,10 @@
#define FILEID_INVALID 0xff
#endif

+#ifndef EFSCORRUPTED
+#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
+#endif
+
#define BUF_SIZE 256
static char event_buf[BUF_SIZE];
int fd_notify;
@@ -63,6 +67,17 @@ static void trigger_fs_abort(void)
MS_REMOUNT|MS_RDONLY, "abort");
}

+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;
@@ -77,6 +92,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-27 06:01:16

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 04/10] syscalls/fanotify20: 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.

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

---
Changes since v1:
- Move defines to header file.
---
testcases/kernel/syscalls/fanotify/fanotify.h | 32 +++++++++++++++++
.../kernel/syscalls/fanotify/fanotify20.c | 35 ++++++++++++++++++-
2 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 8828b53532a2..58e30aaf00bc 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])
@@ -403,4 +414,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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 7a522aad4386..6074d449ae63 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -42,10 +42,32 @@ int fd_notify;

static struct test_case {
char *name;
+ int error;
+ unsigned int error_count;
void (*trigger_error)(void);
} testcases[] = {
};

+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;
@@ -68,6 +90,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");
@@ -77,7 +101,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-27 06:01:16

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 05/10] syscalls/fanotify20: 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).

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

---
Changes since v1:
- Move defines to header file.
- Use 0-len FH for sb error
---
testcases/kernel/syscalls/fanotify/fanotify.h | 4 ++
.../kernel/syscalls/fanotify/fanotify20.c | 63 +++++++++++++++++++
2 files changed, 67 insertions(+)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 58e30aaf00bc..9bff3cf1a3fe 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -435,4 +435,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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 6074d449ae63..220cd51419e8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -34,20 +34,61 @@
#ifdef HAVE_SYS_FANOTIFY_H
#include "fanotify.h"

+#ifndef FILEID_INVALID
+#define FILEID_INVALID 0xff
+#endif
+
#define BUF_SIZE 256
static char event_buf[BUF_SIZE];
int fd_notify;

#define MOUNT_PATH "test_mnt"

+/* These expected FIDs are common to multiple tests */
+static struct fanotify_fid_t null_fid;
+
static struct test_case {
char *name;
int error;
unsigned int error_count;
+ struct fanotify_fid_t *fid;
void (*trigger_error)(void);
} testcases[] = {
};

+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)
{
@@ -91,6 +132,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) {
@@ -109,6 +151,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);
}
@@ -125,12 +175,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-27 06:01:16

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 07/10] syscalls/fanotify20: 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.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 7c4b01720654..298bb303a810 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -43,9 +43,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 do_debugfs_request(const char *dev, char *request)
{
@@ -182,6 +185,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
@@ -201,6 +216,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-27 06:01:16

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 06/10] syscalls/fanotify20: 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.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 220cd51419e8..7c4b01720654 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -47,6 +47,13 @@ int fd_notify;
/* These expected FIDs are common to multiple tests */
static struct fanotify_fid_t null_fid;

+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;
@@ -216,7 +223,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-27 06:01:18

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 08/10] syscalls/fanotify20: Test event after filesystem abort

This test monitors the error triggered after a file system abort. It
works by forcing a remount with the option "abort". This is an error
not related to a file so it is reported against the superblock with a
zero size fh.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 298bb303a810..5c5ee3c6fb74 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -57,6 +57,12 @@ static void do_debugfs_request(const char *dev, char *request)
SAFE_CMD(cmd, NULL, NULL);
}

+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;
int error;
@@ -64,6 +70,13 @@ static struct test_case {
struct fanotify_fid_t *fid;
void (*trigger_error)(void);
} testcases[] = {
+ {
+ .name = "Trigger abort",
+ .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,
--
2.33.0

2021-10-27 06:01:18

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 10/10] syscalls/fanotify20: 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.

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

diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
index 7bcddcaa98cb..0083a018f2c6 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify20.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
@@ -78,6 +78,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;
@@ -99,6 +111,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-27 06:01:56

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: [PATCH v2 02/10] 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]>
---
testcases/kernel/syscalls/fanotify/fanotify.h | 28 +++++++++++++++++--
.../kernel/syscalls/fanotify/fanotify03.c | 4 +--
.../kernel/syscalls/fanotify/fanotify10.c | 3 +-
.../kernel/syscalls/fanotify/fanotify12.c | 3 +-
4 files changed, 31 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index c67db3117e29..b2b56466d028 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -266,14 +266,26 @@ 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 = fanotify_init(init_flags, O_RDONLY);

- if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
+ if (fd < 0) {
+ if (errno == EINVAL) {
+ rval = -1;
+ } else {
+ tst_brk(TBROK | TERRNO,
+ "fanotify_init (%d, FAN_CLASS_CONTENT, ..., 0_RDONLY", fd);
+ }
+ goto out;
+ }
+
+ if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
if (errno == EINVAL) {
rval = -1;
} else {
@@ -284,6 +296,7 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask)

SAFE_CLOSE(fd);

+out:
return rval;
}

@@ -378,4 +391,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..d863ae1a7d6d 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_CONTENT, 0);

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

2021-10-27 16:28:48

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 01/10] syscalls: fanotify: Add macro to require specific mark types

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Like done for init flags and event types, and a macro to require a
> specific mark type.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

Reviewed-by: Amir Goldstein <[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-27 17:45:10

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 02/10] syscalls: fanotify: Add macro to require specific events

On Tue, Oct 26, 2021 at 9:43 PM 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]>
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 28 +++++++++++++++++--
> .../kernel/syscalls/fanotify/fanotify03.c | 4 +--
> .../kernel/syscalls/fanotify/fanotify10.c | 3 +-
> .../kernel/syscalls/fanotify/fanotify12.c | 3 +-
> 4 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index c67db3117e29..b2b56466d028 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -266,14 +266,26 @@ 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 = fanotify_init(init_flags, O_RDONLY);
>
> - if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
> + if (fd < 0) {
> + if (errno == EINVAL) {
> + rval = -1;
> + } else {
> + tst_brk(TBROK | TERRNO,
> + "fanotify_init (%d, FAN_CLASS_CONTENT, ..., 0_RDONLY", fd);

init flags in the print are incorrect, but I don't think you should
bother with that.
I think you should leave SAFE_FANOTIFY_INIT, because none of the existing
tests are going to fail the init flags and seems like your new test is
going to use the
REQUIRE_FANOTIFY_EVENTS_SUPPORTED_ON_FS macro that will fail
on unsupported init flags (with correct print) anyway.

> + }
> + goto out;
> + }
> +
> + if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> if (errno == EINVAL) {
> rval = -1;
> } else {
> @@ -284,6 +296,7 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask)
>
> SAFE_CLOSE(fd);
>
> +out:
> return rval;
> }
>
> @@ -378,4 +391,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..d863ae1a7d6d 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_CONTENT, 0);
>

The hardcoded FAN_CLASS_CONTENT was the common flag to use for all
test, but this
test in particular does not use FAN_CLASS_CONTENT it uses FAN_CLASS_NOTIFY, so
let's express the requirements accurately.

Thanks,
Amir.

2021-10-27 17:45:10

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 03/10] syscalls/fanotify20: Introduce helpers for FAN_FS_ERROR test

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> fanotify20 is a new test validating the FAN_FS_ERROR file system error
> event. This adds some basic structure for the next patches.
>
> The strategy for error reporting testing in fanotify20 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.
> ---
> testcases/kernel/syscalls/fanotify/.gitignore | 1 +
> testcases/kernel/syscalls/fanotify/fanotify.h | 3 +
> .../kernel/syscalls/fanotify/fanotify20.c | 128 ++++++++++++++++++
> 3 files changed, 132 insertions(+)
> create mode 100644 testcases/kernel/syscalls/fanotify/fanotify20.c
>
> diff --git a/testcases/kernel/syscalls/fanotify/.gitignore b/testcases/kernel/syscalls/fanotify/.gitignore
> index 9554b16b196e..c99e6fff76d6 100644
> --- a/testcases/kernel/syscalls/fanotify/.gitignore
> +++ b/testcases/kernel/syscalls/fanotify/.gitignore
> @@ -17,4 +17,5 @@
> /fanotify17
> /fanotify18
> /fanotify19
> +/fanotify20
> /fanotify_child
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index b2b56466d028..8828b53532a2 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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> new file mode 100644
> index 000000000000..7a522aad4386
> --- /dev/null
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -0,0 +1,128 @@
> +// 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 struct test_case {
> + char *name;
> + void (*trigger_error)(void);
> +} testcases[] = {
> +};
> +


Does LTP accept .tcnt = 0 gracefully?
or maybe LTP project does not care much about failing tests during bisection?

> +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;
> +
> + tcase->trigger_error();
> +
> + read_len = SAFE_READ(0, fd_notify, event_buf, BUF_SIZE);
> +
> + 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);
> +
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD|FAN_MARK_FILESYSTEM,
> + FAN_FS_ERROR, AT_FDCWD, MOUNT_PATH);

I think it is better to have the mark add/remove inside do_test
This way when running fanotify -i 10 (which testers do)
we also get test coverage for add/remove of mark with FS_ERROR mask.

Thanks,
Amir.

2021-10-27 17:46:15

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] syscalls/fanotify20: Validate the generic error info

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
>

After fixing and testing configure.ac you may add:

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

> ---
> Changes since v1:
> - Move defines to header file.
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 32 +++++++++++++++++
> .../kernel/syscalls/fanotify/fanotify20.c | 35 ++++++++++++++++++-
> 2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 8828b53532a2..58e30aaf00bc 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 */

Need to add in configure.ac:

AC_CHECK_TYPES([struct fanotify_event_info_error, struct
fanotify_event_info_header],,,[#include <sys/fanotify.h>])

(not tested)

> +
> /* 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])
> @@ -403,4 +414,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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 7a522aad4386..6074d449ae63 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -42,10 +42,32 @@ int fd_notify;
>
> static struct test_case {
> char *name;
> + int error;
> + unsigned int error_count;
> void (*trigger_error)(void);
> } testcases[] = {
> };
>
> +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;
> @@ -68,6 +90,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");
> @@ -77,7 +101,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-27 17:48:10

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] syscalls/fanotify20: Create a corrupted file

On Tue, Oct 26, 2021 at 9:44 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

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

> ---
> testcases/kernel/syscalls/fanotify/fanotify20.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 7c4b01720654..298bb303a810 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -43,9 +43,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 do_debugfs_request(const char *dev, char *request)
> {
> @@ -182,6 +185,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
> @@ -201,6 +216,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-27 17:48:13

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 05/10] syscalls/fanotify20: Validate incoming FID in FAN_FS_ERROR

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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).
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

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

Except maybe move define of FILEID_INVALID to header.

>
> ---
> Changes since v1:
> - Move defines to header file.
> - Use 0-len FH for sb error
> ---
> testcases/kernel/syscalls/fanotify/fanotify.h | 4 ++
> .../kernel/syscalls/fanotify/fanotify20.c | 63 +++++++++++++++++++
> 2 files changed, 67 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 58e30aaf00bc..9bff3cf1a3fe 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -435,4 +435,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/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 6074d449ae63..220cd51419e8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -34,20 +34,61 @@
> #ifdef HAVE_SYS_FANOTIFY_H
> #include "fanotify.h"
>
> +#ifndef FILEID_INVALID
> +#define FILEID_INVALID 0xff
> +#endif
> +
> #define BUF_SIZE 256
> static char event_buf[BUF_SIZE];
> int fd_notify;
>
> #define MOUNT_PATH "test_mnt"
>
> +/* These expected FIDs are common to multiple tests */
> +static struct fanotify_fid_t null_fid;
> +
> static struct test_case {
> char *name;
> int error;
> unsigned int error_count;
> + struct fanotify_fid_t *fid;
> void (*trigger_error)(void);
> } testcases[] = {
> };
>
> +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)
> {
> @@ -91,6 +132,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) {
> @@ -109,6 +151,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);
> }
> @@ -125,12 +175,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-27 17:48:13

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] syscalls/fanotify20: Support submission of debugfs commands

On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

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

> ---
> changes since v1:
> - Add .needs_cmds to require debugfs
> ---
> testcases/kernel/syscalls/fanotify/fanotify20.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 220cd51419e8..7c4b01720654 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -47,6 +47,13 @@ int fd_notify;
> /* These expected FIDs are common to multiple tests */
> static struct fanotify_fid_t null_fid;
>
> +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;
> @@ -216,7 +223,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-27 21:20:39

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 09/10] syscalls/fanotify20: Test file event with broken inode

On Tue, Oct 26, 2021 at 9:44 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

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

> ---
> .../kernel/syscalls/fanotify/fanotify20.c | 22 +++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 5c5ee3c6fb74..7bcddcaa98cb 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -38,6 +38,10 @@
> #define FILEID_INVALID 0xff
> #endif
>
> +#ifndef EFSCORRUPTED
> +#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
> +#endif
> +
> #define BUF_SIZE 256
> static char event_buf[BUF_SIZE];
> int fd_notify;
> @@ -63,6 +67,17 @@ static void trigger_fs_abort(void)
> MS_REMOUNT|MS_RDONLY, "abort");
> }
>
> +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;
> @@ -77,6 +92,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-27 21:21:10

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 10/10] syscalls/fanotify20: Test capture of multiple errors

On Tue, Oct 26, 2021 at 9:44 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> 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.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>

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

> ---
> .../kernel/syscalls/fanotify/fanotify20.c | 26 +++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 7bcddcaa98cb..0083a018f2c6 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -78,6 +78,18 @@ static void tcase2_trigger_lookup(void)
> ret, BAD_DIR, errno, EUCLEAN);
> }
>
> +static void tcase3_trigger(void)
> +{
> + trigger_fs_abort();
> + tcase2_trigger_lookup();

So after remount,abort filesystem operations can still be executed?
Then I guess my comment from the previous patch about running the test in a loop
is not relevant?

Thanks,
Amir.

2021-10-27 21:21:17

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 08/10] syscalls/fanotify20: Test event after filesystem abort

On Tue, Oct 26, 2021 at 9:44 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> This test monitors the error triggered after a file system abort. It
> works by forcing a remount with the option "abort". This is an error
> not related to a file so it is reported against the superblock with a
> zero size fh.
>
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> ---
> testcases/kernel/syscalls/fanotify/fanotify20.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c
> index 298bb303a810..5c5ee3c6fb74 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
> @@ -57,6 +57,12 @@ static void do_debugfs_request(const char *dev, char *request)
> SAFE_CMD(cmd, NULL, NULL);
> }
>
> +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;
> int error;
> @@ -64,6 +70,13 @@ static struct test_case {
> struct fanotify_fid_t *fid;
> void (*trigger_error)(void);
> } testcases[] = {
> + {
> + .name = "Trigger abort",
> + .trigger_error = &trigger_fs_abort,
> + .error_count = 1,
> + .error = ESHUTDOWN,
> + .fid = &null_fid,
> + },
> };
>

I suppose you did not try to run fanotify20 -i 10 ...?
I guess you will need to end the setup() stage with unmounted fs and perform:
mount; fanotify_init; fanotify_mark; at beginning of do_test()
finishing do_test() with closing fanotify fd and unmount.
I never checked it there are pre-test/post-test callbacks available in LTP,
but setup/clean are called at start/end of test loop.

Thanks,
Amir.

2021-10-27 21:21:38

by Amir Goldstein

[permalink] [raw]
Subject: Re: [PATCH v2 04/10] syscalls/fanotify20: Validate the generic error info

On Wed, Oct 27, 2021 at 9:43 AM Amir Goldstein <[email protected]> wrote:
>
> On Tue, Oct 26, 2021 at 9:43 PM Gabriel Krisman Bertazi
> <[email protected]> wrote:
> >
> > 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.
> >
> > Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> >
>
> After fixing and testing configure.ac you may add:
>
> Reviewed-by: Amir Goldstein <[email protected]>
>
> > ---
> > Changes since v1:
> > - Move defines to header file.
> > ---
> > testcases/kernel/syscalls/fanotify/fanotify.h | 32 +++++++++++++++++
> > .../kernel/syscalls/fanotify/fanotify20.c | 35 ++++++++++++++++++-
> > 2 files changed, 66 insertions(+), 1 deletion(-)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> > index 8828b53532a2..58e30aaf00bc 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 */
>
> Need to add in configure.ac:
>
> AC_CHECK_TYPES([struct fanotify_event_info_error, struct
> fanotify_event_info_header],,,[#include <sys/fanotify.h>])
>
> (not tested)

According to Matthew's pidfd patches the syntax should be:

AC_CHECK_TYPES([struct fanotify_event_info_fid, struct
fanotify_event_info_header, struct fanotify_event_info_pidfd, struct
fanotify_event_info_error],,,[#include <sys/fanotify.h>])

Thanks,
Amir.

2021-10-29 15:04:35

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH v2 10/10] syscalls/fanotify20: Test capture of multiple errors

Amir Goldstein <[email protected]> writes:

> On Tue, Oct 26, 2021 at 9:44 PM Gabriel Krisman Bertazi
>> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c
>> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c
>> @@ -78,6 +78,18 @@ static void tcase2_trigger_lookup(void)
>> ret, BAD_DIR, errno, EUCLEAN);
>> }
>>
>> +static void tcase3_trigger(void)
>> +{
>> + trigger_fs_abort();
>> + tcase2_trigger_lookup();
>
> So after remount,abort filesystem operations can still be executed?
> Then I guess my comment from the previous patch about running the test in a loop
> is not relevant?

Hi Amir,

As you mentioned here, -i works fine. Abort will remount with
MS_RDONLY, and this doesn't affect the existing tests. Future tests
that write to the file system inside .trigger_error() would require the
umount-mount cycle but, since the goal is testing fanotify and not
specific fs errors, I think we don't need the added complexity of such
tests.

Output of '-i #' always pass:

root@test-box:~/ltp/testcases/kernel/syscalls/fanotify# ./fanotify20 -i 5
tst_device.c:88: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:932: TINFO: Formatting /dev/loop0 with ext4 opts='' extra opts=''
mke2fs 1.46.4 (18-Aug-2021)
tst_test.c:1363: TINFO: Timeout per run is 0h 05m 00s
fanotify.h:252: TINFO: fid(test_mnt) = 469af9fc.8ced5767.2.0.0...
fanotify.h:252: TINFO: fid(test_mnt/internal_dir/bad_dir) = 469af9fc.8ced5767.8002.acd05469.0...
debugfs 1.46.4 (18-Aug-2021)
fanotify20.c:234: TPASS: Successfully received: Trigger abort
fanotify20.c:234: TPASS: Successfully received: Lookup of inode with invalid mode
fanotify20.c:234: TPASS: Successfully received: Multiple error submission
fanotify20.c:234: TPASS: Successfully received: Multiple error submission 2
fanotify20.c:234: TPASS: Successfully received: Trigger abort
fanotify20.c:234: TPASS: Successfully received: Lookup of inode with invalid mode
fanotify20.c:234: TPASS: Successfully received: Multiple error submission
fanotify20.c:234: TPASS: Successfully received: Multiple error submission 2
fanotify20.c:234: TPASS: Successfully received: Trigger abort
fanotify20.c:234: TPASS: Successfully received: Lookup of inode with invalid mode
fanotify20.c:234: TPASS: Successfully received: Multiple error submission
fanotify20.c:234: TPASS: Successfully received: Multiple error submission 2
fanotify20.c:234: TPASS: Successfully received: Trigger abort
fanotify20.c:234: TPASS: Successfully received: Lookup of inode with invalid mode
fanotify20.c:234: TPASS: Successfully received: Multiple error submission
fanotify20.c:234: TPASS: Successfully received: Multiple error submission 2
fanotify20.c:234: TPASS: Successfully received: Trigger abort
fanotify20.c:234: TPASS: Successfully received: Lookup of inode with invalid mode
fanotify20.c:234: TPASS: Successfully received: Multiple error submission
fanotify20.c:234: TPASS: Successfully received: Multiple error submission 2

Summary:
passed 20
failed 0
broken 0
skipped 0
warnings 0

Thanks,

--
Gabriel Krisman Bertazi