* [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
@ 2020-11-26 21:41 Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 1/6] fanotify12: Drop incorrect hint Petr Vorel
` (7 more replies)
0 siblings, 8 replies; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
Hi Amir,
another cleanup version, hopefully the last.
Sorry for lengthy comments.
Changes v3->v4:
* fixed unwanted tst_brk() (quitting the test too early). In the end it
was problem just in fanotify01.c and fanotify03.c.
In fanotify13.c it was already
tst_test.c:1316: TINFO: Testing on exfat
fanotify.h:209: TCONF: filesystem exfat does not support file handles
...
tst_test.c:859: TINFO: Trying FUSE...
fanotify13.c:161: TINFO: Test #0: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
fanotify13.c:161: TINFO: Test #1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
fanotify13.c:161: TINFO: Test #2: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
fanotify13.c:161: TINFO: Test #3: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
fanotify13.c:161: TINFO: Test #4: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
fanotify13.c:161: TINFO: Test #5: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
* new commit fanotify: Check for FAN_REPORT_FID support on fs
This one I hesitated, whether keep EOPNOTSUPP also as fallback in fanotify.h
On all but one test (fanotify01.c) FAN_REPORT_FID was used in all tests.
The only check I kept untouched is this one from fanotify16.c:
fd_notify = fanotify_init(group->flag, 0);
if (fd_notify == -1) {
if (errno == EINVAL) {
tst_res(TCONF,
"%s not supported by kernel", group->name);
return;
}
tst_brk(TBROK | TERRNO,
"fanotify_init(%s, 0) failed", group->name);
}
Because this:
fanotify16.c:160: TINFO: Test #0: FAN_REPORT_DFID_NAME monitor filesystem for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME not supported by kernel
fanotify16.c:160: TINFO: Test #1: FAN_REPORT_DFID_NAME monitor directories for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME not supported by kernel
fanotify16.c:160: TINFO: Test #2: FAN_REPORT_DIR_FID monitor filesystem for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DIR_FID not supported by kernel
fanotify16.c:160: TINFO: Test #3: FAN_REPORT_DIR_FID monitor directories for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DIR_FID not supported by kernel
fanotify16.c:160: TINFO: Test #4: FAN_REPORT_DFID_FID monitor filesystem for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_FID not supported by kernel
fanotify16.c:160: TINFO: Test #5: FAN_REPORT_DFID_FID monitor directories for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_FID not supported by kernel
fanotify16.c:160: TINFO: Test #6: FAN_REPORT_DFID_NAME_FID monitor filesystem for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME_FID not supported by kernel
fanotify16.c:160: TINFO: Test #7: FAN_REPORT_DFID_NAME_FID monitor directories for create/delete/move/open/close
fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME_FID not supported by kernel
would be replaced by single message which is misleading:
fanotify16.c:163: TCONF: FAN_REPORT_NAME not supported in kernel?
if I use
fd_notify = SAFE_FANOTIFY_INIT(group->flag, 0);
+ that problem with skipping what we don't want to skip (although here
are skipped all and on old kernel I get
fanotify.h:342: TCONF: FAN_REPORT_FID not supported in kernel?
and on new kernel problematic fs are skipped anyway:
fanotify.h:363: TCONF: FAN_REPORT_FID not supported on exfat filesystem
Feel free to suggest what to do or simply send a following patch.
In following commit "fanotify: Introduce SAFE_FANOTIFY_MARK() macro".
I'd prefer to keep it (fallback if we forget to add a check), but probably instead of
"some FAN_REPORT_* flag not supported on %s filesystem",
there should be
"FAN_REPORT_FID flag not supported on %s filesystem",
But on my machine with 5.10.0-rc4-1.gea0f69f-default, this failed on FAN_REPORT_DFID_NAME
(synonym for (FAN_REPORT_DIR_FID|FAN_REPORT_NAME) => no FAN_REPORT_FID,
so this would be misleading:
fanotify16.c:160: TINFO: Test #0: FAN_REPORT_DFID_NAME monitor filesystem for create/delete/move/open/close
fanotify16.c:177: TCONF: FAN_REPORT_FID not supported on exfat filesystem
* more cleanup in commit "fanotify: Check FAN_REPORT_{FID,NAME} support"
Other changes (suggested mostly by Cyril):
* rename s/support_exec_events/exec_events_unsupported/
* Add "require_" prefix for functions which use tst_brk()
* use tst_res_() and tst_brk_() for safe_*() functions
BTW fanotify16.c use .dev_fs_flags = TST_FS_SKIP_FUSE, this could be
added also into fanotify13.c (it'd avoid running ntfs).
If LTP had something like TST_FS_SKIP_MICROSOFT (TST_FS_SKIP_FUSE +
exfat), could get rid of "FAN_REPORT_FID not supported on foo
filesystem" testing. But it's already implemented, so it's just a note
to be ignored.
Kind regards,
Petr
Petr Vorel (6):
fanotify12: Drop incorrect hint
fanotify: Handle supported features checks in setup()
fanotify: Check for FAN_REPORT_FID support on fs
fanotify: Introduce SAFE_FANOTIFY_MARK() macro
fanotify: Check FAN_REPORT_{FID,NAME} support
fanotify: Add a pedantic check for return value
testcases/kernel/syscalls/fanotify/fanotify.h | 173 +++++++++++++++---
.../kernel/syscalls/fanotify/fanotify01.c | 73 ++------
.../kernel/syscalls/fanotify/fanotify02.c | 22 +--
.../kernel/syscalls/fanotify/fanotify03.c | 55 ++----
.../kernel/syscalls/fanotify/fanotify04.c | 32 +---
.../kernel/syscalls/fanotify/fanotify05.c | 9 +-
.../kernel/syscalls/fanotify/fanotify06.c | 21 +--
.../kernel/syscalls/fanotify/fanotify07.c | 17 +-
.../kernel/syscalls/fanotify/fanotify09.c | 19 +-
.../kernel/syscalls/fanotify/fanotify10.c | 44 ++---
.../kernel/syscalls/fanotify/fanotify11.c | 5 +-
.../kernel/syscalls/fanotify/fanotify12.c | 57 ++----
.../kernel/syscalls/fanotify/fanotify13.c | 49 +----
.../kernel/syscalls/fanotify/fanotify15.c | 35 +---
.../kernel/syscalls/fanotify/fanotify16.c | 26 +--
15 files changed, 249 insertions(+), 388 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 1/6] fanotify12: Drop incorrect hint
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup() Petr Vorel
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
hint "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?"
is wrong here (fanotify12 does not use FAN_ACCESS_PERM).
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v3->v4:
Removed second CONFIG_FANOTIFY_ACCESS_PERMISSIONS
testcases/kernel/syscalls/fanotify/fanotify12.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index fcb7ec0d3..fea7cb607 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -146,10 +146,6 @@ static int setup_mark(unsigned int n)
"FAN_OPEN_EXEC not supported in "
"kernel?");
return -1;
- } else if (errno == EINVAL) {
- tst_brk(TCONF | TERRNO,
- "CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
- "not configured in kernel?");
}else {
tst_brk(TBROK | TERRNO,
"fanotify_mark(%d, FAN_MARK_ADD | %s, "
@@ -173,11 +169,6 @@ static int setup_mark(unsigned int n)
"FAN_OPEN_EXEC not supported "
"in kernel?");
return -1;
- } else if (errno == EINVAL) {
- tst_brk(TCONF | TERRNO,
- "CONFIG_FANOTIFY_ACCESS_"
- "PERMISSIONS not configured in "
- "kernel?");
} else {
tst_brk(TBROK | TERRNO,
"fanotify_mark (%d, "
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup()
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 1/6] fanotify12: Drop incorrect hint Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-27 13:20 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs Petr Vorel
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
Create 2 helpers which are used in various tests in setup() to check for
FAN_ACCESS_PERM enablement (caused by disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS)
or FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM support in kernel.
That helps to further code simplification in next commit.
Due this change test with FAN_OPEN_EXEC_PERM in fanotify03.c no longer
needs to be a first test.
Also rename variable s/support_exec_events/exec_events_unsupported/
for readability.
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v3->v4:
* rename s/support_exec_events/exec_events_unsupported/
* Add "require_" prefix for functions which use tst_brk()
testcases/kernel/syscalls/fanotify/fanotify.h | 41 +++++++++++++++
.../kernel/syscalls/fanotify/fanotify03.c | 37 +++++--------
.../kernel/syscalls/fanotify/fanotify07.c | 14 ++---
.../kernel/syscalls/fanotify/fanotify10.c | 8 +++
.../kernel/syscalls/fanotify/fanotify12.c | 52 ++++++++-----------
5 files changed, 89 insertions(+), 63 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 0afbc02aa..413034336 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -60,6 +60,7 @@ int safe_fanotify_init(const char *file, const int lineno,
return rval;
}
+
#define SAFE_FANOTIFY_INIT(fan, mode) \
safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
@@ -242,4 +243,44 @@ static inline void fanotify_save_fid(const char *path,
#define INIT_FANOTIFY_MARK_TYPE(t) \
{ FAN_MARK_ ## t, "FAN_MARK_" #t }
+static inline void require_fanotify_access_permissions_supported_by_kernel(void)
+{
+ int fd;
+
+ fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+ if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, ".") < 0) {
+ if (errno == EINVAL) {
+ tst_brk(TCONF | TERRNO,
+ "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not configured in kernel?");
+ } else {
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, \".\") failed", fd);
+ }
+ }
+
+ SAFE_CLOSE(fd);
+}
+
+static inline int fanotify_exec_events_supported_by_kernel(uint64_t mask)
+{
+ int fd;
+ int rval = 0;
+
+ fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
+
+ if (fanotify_mark(fd, FAN_MARK_ADD, mask, AT_FDCWD, ".") < 0) {
+ if (errno == EINVAL) {
+ rval = -1;
+ } else {
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
+ }
+ }
+
+ SAFE_CLOSE(fd);
+
+ return rval;
+}
+
#endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index 1ef1c206b..ccb9a4799 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -48,18 +48,13 @@ static volatile int fd_notify;
static pid_t child_pid;
static char event_buf[EVENT_BUF_LEN];
-static int support_exec_events;
+static int exec_events_unsupported;
struct event {
unsigned long long mask;
unsigned int response;
};
-/*
- * Ensure to keep the first FAN_OPEN_EXEC_PERM test case before the first
- * MARK_TYPE(FILESYSTEM) in order to allow for correct detection between
- * exec events not supported and filesystem marks not supported.
- */
static struct tcase {
const char *tname;
struct fanotify_mark_type mark;
@@ -212,28 +207,23 @@ static int setup_mark(unsigned int n)
char *const files[] = {fname, FILE_EXEC_PATH};
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+ if (exec_events_unsupported && tc->mask & FAN_OPEN_EXEC_PERM) {
+ tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC_PERM not supported in kernel?");
+ return -1;
+ }
+
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
for (; i < ARRAY_SIZE(files); i++) {
if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
tc->mask, AT_FDCWD, files[i]) < 0) {
if (errno == EINVAL &&
- (tc->mask & FAN_OPEN_EXEC_PERM &&
- !support_exec_events)) {
- tst_res(TCONF,
- "FAN_OPEN_EXEC_PERM not supported in "
- "kernel?");
- return -1;
- } else if (errno == EINVAL &&
mark->flag == FAN_MARK_FILESYSTEM) {
tst_res(TCONF,
"FAN_MARK_FILESYSTEM not supported in "
"kernel?");
return -1;
- } else if (errno == EINVAL) {
- tst_brk(TCONF | TERRNO,
- "CONFIG_FANOTIFY_ACCESS_PERMISSIONS "
- "not configured in kernel?");
} else {
tst_brk(TBROK | TERRNO,
"fanotify_mark(%d, FAN_MARK_ADD | %s, "
@@ -241,14 +231,6 @@ static int setup_mark(unsigned int n)
"AT_FDCWD, %s) failed.",
fd_notify, mark->name, fname);
}
- } else {
- /*
- * To distinguish between perm not supported, exec
- * events not supported and filesystem mark not
- * supported.
- */
- if (tc->mask & FAN_OPEN_EXEC_PERM)
- support_exec_events = 1;
}
}
@@ -347,6 +329,11 @@ static void test_fanotify(unsigned int n)
static void setup(void)
{
+
+ require_fanotify_access_permissions_supported_by_kernel();
+
+ exec_events_unsupported = fanotify_exec_events_supported_by_kernel(FAN_OPEN_EXEC_PERM);
+
sprintf(fname, MOUNT_PATH"/fname_%d", getpid());
SAFE_FILE_PRINTF(fname, "1");
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index c2e185710..533c880d0 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -106,15 +106,9 @@ static int setup_instance(void)
if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
fname) < 0) {
close(fd);
- if (errno == EINVAL) {
- tst_brk(TCONF | TERRNO,
- "CONFIG_FANOTIFY_ACCESS_PERMISSIONS not "
- "configured in kernel?");
- } else {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, "
- "AT_FDCWD, %s) failed.", fd, fname);
- }
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, %s) failed.",
+ fd, fname);
}
return fd;
@@ -195,6 +189,8 @@ static void test_fanotify(void)
static void setup(void)
{
+ require_fanotify_access_permissions_supported_by_kernel();
+
sprintf(fname, "fname_%d", getpid());
SAFE_FILE_PRINTF(fname, "%s", fname);
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 90cf5cb5f..82f8a7774 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -64,6 +64,7 @@ static unsigned int fanotify_class[] = {
static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
static char event_buf[EVENT_BUF_LEN];
+static int exec_events_unsupported;
#define MOUNT_PATH "fs_mnt"
#define MNT2_PATH "mntpoint"
@@ -451,6 +452,11 @@ static void test_fanotify(unsigned int n)
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+ if (exec_events_unsupported && tc->expected_mask_with_ignore & FAN_OPEN_EXEC) {
+ tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
+ return;
+ }
+
if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
" has undefined behavior on kernel < 5.9");
@@ -535,6 +541,8 @@ cleanup:
static void setup(void)
{
+ exec_events_unsupported = fanotify_exec_events_supported_by_kernel(FAN_OPEN_EXEC);
+
/* Create another bind mount@another path for generating events */
SAFE_MKDIR(MNT2_PATH, 0755);
SAFE_MOUNT(MOUNT_PATH, MNT2_PATH, "none", MS_BIND, NULL);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index fea7cb607..dde1a3aea 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -39,6 +39,7 @@ static char fname[BUF_SIZE];
static volatile int fd_notify;
static volatile int complete;
static char event_buf[EVENT_BUF_LEN];
+static int exec_events_unsupported;
static struct test_case_t {
const char *tname;
@@ -135,26 +136,25 @@ static int setup_mark(unsigned int n)
const char *const files[] = {fname, TEST_APP};
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
+ if (exec_events_unsupported && tc->mask & FAN_OPEN_EXEC) {
+ tst_res(TCONF | TERRNO, "FAN_OPEN_EXEC not supported in kernel?");
+ return -1;
+ }
+
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
for (; i < ARRAY_SIZE(files); i++) {
/* Setup normal mark on object */
if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
tc->mask, AT_FDCWD, files[i]) < 0) {
- if (errno == EINVAL && tc->mask & FAN_OPEN_EXEC) {
- tst_res(TCONF,
- "FAN_OPEN_EXEC not supported in "
- "kernel?");
- return -1;
- }else {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s, "
- "%llx, AT_FDCWD, %s) failed",
- fd_notify,
- mark->name,
- tc->mask,
- files[i]);
- }
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark(%d, FAN_MARK_ADD | %s, "
+ "%llx, AT_FDCWD, %s) failed",
+ fd_notify,
+ mark->name,
+ tc->mask,
+ files[i]);
}
/* Setup ignore mark on object */
@@ -163,21 +163,13 @@ static int setup_mark(unsigned int n)
| FAN_MARK_IGNORED_MASK,
tc->ignore_mask, AT_FDCWD,
files[i]) < 0) {
- if (errno == EINVAL &&
- tc->ignore_mask & FAN_OPEN_EXEC) {
- tst_res(TCONF,
- "FAN_OPEN_EXEC not supported "
- "in kernel?");
- return -1;
- } else {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, "
- "FAN_MARK_ADD | %s "
- "| FAN_MARK_IGNORED_MASK, "
- "%llx, AT_FDCWD, %s) failed",
- fd_notify, mark->name,
- tc->ignore_mask, files[i]);
- }
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, "
+ "FAN_MARK_ADD | %s "
+ "| FAN_MARK_IGNORED_MASK, "
+ "%llx, AT_FDCWD, %s) failed",
+ fd_notify, mark->name,
+ tc->ignore_mask, files[i]);
}
}
}
@@ -244,6 +236,8 @@ cleanup:
static void do_setup(void)
{
+ exec_events_unsupported = fanotify_exec_events_supported_by_kernel(FAN_OPEN_EXEC);
+
sprintf(fname, "fname_%d", getpid());
SAFE_FILE_PRINTF(fname, "1");
}
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 1/6] fanotify12: Drop incorrect hint Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup() Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-27 13:47 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
This is related to kernel fix
a8b13aa20afb ("fanotify: enable FAN_REPORT_FID init flag")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
New in v4.
Maybe it'd deserve better commit message.
There might be even more cleanup: not sure if nofid_fd in fanotify13.c
is required. According to the description is probably required:
static void do_setup(void)
{
require_fanotify_fan_report_fid_supported_on_fs(MOUNT_PATH);
nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
/* Create file and directory objects for testing */
create_objects();
/*
* Create a mark on first inode without FAN_REPORT_FID, to test
* uninitialized connector->fsid cache. This mark remains for all test
* cases and is not expected to get any events (no writes in this test).
*/
SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
FILE_PATH_ONE);
/* Get the filesystem fsid and file handle for each created object */
get_object_stats();
testcases/kernel/syscalls/fanotify/fanotify.h | 31 +++++++++++++++++++
.../kernel/syscalls/fanotify/fanotify01.c | 9 +++++-
.../kernel/syscalls/fanotify/fanotify13.c | 4 ++-
.../kernel/syscalls/fanotify/fanotify15.c | 6 ++--
.../kernel/syscalls/fanotify/fanotify16.c | 6 +---
5 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 413034336..c690b82d3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -283,4 +283,35 @@ static inline int fanotify_exec_events_supported_by_kernel(uint64_t mask)
return rval;
}
+static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
+{
+ int fd;
+ int rval = 0;
+
+ fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
+
+ if (fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
+ FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
+ AT_FDCWD, fname) < 0) {
+ if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
+ rval = -1;
+ } else {
+ tst_brk(TBROK | TERRNO,
+ "fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
+ }
+ }
+
+ SAFE_CLOSE(fd);
+
+ return rval;
+}
+
+static inline void require_fanotify_fan_report_fid_supported_on_fs(const char *fname)
+{
+ if (fanotify_fan_report_fid_supported_on_fs(fname) != 0) {
+ tst_brk(TCONF, "FAN_REPORT_FID not supported on %s filesystem",
+ tst_device->fs_type);
+ }
+}
+
#endif /* __FANOTIFY_H__ */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 03e453f41..1e99a5dc7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -74,6 +74,7 @@ static struct tcase {
static char fname[BUF_SIZE];
static char buf[BUF_SIZE];
static int fd_notify;
+static int fan_report_fid_unsupported;
static unsigned long long event_set[EVENT_MAX];
@@ -88,6 +89,12 @@ static void test_fanotify(unsigned int n)
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+ if (fan_report_fid_unsupported && (tc->init_flags & FAN_REPORT_FID)) {
+ tst_res(TCONF | TERRNO, "FAN_REPORT_FID not supported on %s filesystem",
+ tst_device->fs_type);
+ return;
+ }
+
fd_notify = fanotify_init(tc->init_flags, O_RDONLY);
if (fd_notify < 0) {
if (errno == EINVAL &&
@@ -363,9 +370,9 @@ static void setup(void)
/* Check for kernel fanotify support */
fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
SAFE_CLOSE(fd);
-
sprintf(fname, MOUNT_PATH"/tfile_%d", getpid());
SAFE_FILE_PRINTF(fname, "1");
+ fan_report_fid_unsupported = fanotify_fan_report_fid_supported_on_fs(fname);
}
static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index c2a21bb66..39caea41e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -281,7 +281,9 @@ out:
static void do_setup(void)
{
- /* Check for kernel fanotify support */
+
+ require_fanotify_fan_report_fid_supported_on_fs(MOUNT_PATH);
+
nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
/* Create file and directory objects for testing */
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index d787a08e3..c3fc4f8ab 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -89,10 +89,6 @@ static void do_test(unsigned int number)
FAN_CREATE | FAN_DELETE | FAN_MOVE |
FAN_MODIFY | FAN_ONDIR,
AT_FDCWD, TEST_DIR) == -1) {
- if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV)
- tst_brk(TCONF,
- "FAN_REPORT_FID not supported on %s "
- "filesystem", tst_device->fs_type);
tst_brk(TBROK | TERRNO,
"fanotify_mark(%d, FAN_MARK_ADD | %s, "
"FAN_CREATE | FAN_DELETE | FAN_MOVE | "
@@ -303,6 +299,8 @@ static void do_setup(void)
}
SAFE_MKDIR(TEST_DIR, 0755);
+
+ require_fanotify_fan_report_fid_supported_on_fs(TEST_DIR);
}
static void do_cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index 7995a1688..e2a1509b0 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -562,11 +562,7 @@ check_match:
static void setup(void)
{
- int fd;
-
- /* Check kernel for fanotify support */
- fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
- SAFE_CLOSE(fd);
+ require_fanotify_fan_report_fid_supported_on_fs(MOUNT_PATH);
sprintf(dname1, "%s/%s", MOUNT_PATH, DIR_NAME1);
sprintf(dname2, "%s/%s", MOUNT_PATH, DIR_NAME2);
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
` (2 preceding siblings ...)
2020-11-26 21:41 ` [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-27 15:17 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support Petr Vorel
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
and function and use it in all tests which use fanotify_mark().
Motivation was not only to simplify the code but also check for
unsupported filesystems which was missing least for exfat on fanotify16,
because filesystem can be set via LTP_DEV_FS_TYPE environment variable
on tests which don't tests all filesystems.
Previous commit added check for FAN_ACCESS_PERM enablement (caused by
disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS) or FAN_OPEN_EXEC and
FAN_OPEN_EXEC_PERM support in kernel, which are tested in setup
functions for relevant tests, thus only FAN_MARK_FILESYSTEM is needed to
check in safe_fanotify_mark().
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v3->v4:
* use tst_res_() and tst_brk_() for safe_*() functions
testcases/kernel/syscalls/fanotify/fanotify.h | 34 ++++++++++++
.../kernel/syscalls/fanotify/fanotify01.c | 53 ++++---------------
.../kernel/syscalls/fanotify/fanotify02.c | 22 ++------
.../kernel/syscalls/fanotify/fanotify03.c | 18 +------
.../kernel/syscalls/fanotify/fanotify04.c | 32 +++--------
.../kernel/syscalls/fanotify/fanotify05.c | 9 +---
.../kernel/syscalls/fanotify/fanotify06.c | 21 ++------
.../kernel/syscalls/fanotify/fanotify07.c | 9 +---
.../kernel/syscalls/fanotify/fanotify09.c | 19 ++-----
.../kernel/syscalls/fanotify/fanotify10.c | 36 ++-----------
.../kernel/syscalls/fanotify/fanotify11.c | 5 +-
.../kernel/syscalls/fanotify/fanotify12.c | 24 ++-------
.../kernel/syscalls/fanotify/fanotify13.c | 33 ++----------
.../kernel/syscalls/fanotify/fanotify15.c | 20 ++-----
.../kernel/syscalls/fanotify/fanotify16.c | 20 ++-----
15 files changed, 88 insertions(+), 267 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index c690b82d3..2a5280636 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -190,6 +190,40 @@ struct fanotify_event_info_fid {
#define MAX_HANDLE_SZ 128
#endif
+static inline int safe_fanotify_mark(const char *file, const int lineno,
+ int fd, unsigned int flags, uint64_t mask,
+ int dfd, const char *pathname)
+{
+ int rval;
+
+ rval = fanotify_mark(fd, flags, mask, dfd, pathname);
+
+ if (rval == -1) {
+ if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
+ tst_brk_(file, lineno, TCONF,
+ "some FAN_REPORT_* flag not supported on %s filesystem",
+ tst_device->fs_type);
+ }
+
+ if (errno == EINVAL && (flags & FAN_MARK_FILESYSTEM)) {
+ tst_brk_(file, lineno, TCONF,
+ "FAN_MARK_FILESYSTEM not supported in kernel?");
+ }
+
+ tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_mark() failed");
+ }
+
+ if (rval < -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "invalid fanotify_mark() return %d", rval);
+ }
+
+ return rval;
+}
+
+#define SAFE_FANOTIFY_MARK(fd, flags, mask, dfd, pathname) \
+ safe_fanotify_mark(__FILE__, __LINE__, (fd), (flags), (mask), (dfd), (pathname))
+
/*
* Helper function used to obtain fsid and file_handle for a given path.
* Used by test files correlated to FAN_REPORT_FID functionality.
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 1e99a5dc7..170428eba 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -108,19 +108,8 @@ static void test_fanotify(unsigned int n)
"failed", tc->init_flags);
}
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
- FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
- AT_FDCWD, fname) < 0) {
- if (errno == EINVAL && mark->flag == FAN_MARK_FILESYSTEM) {
- tst_res(TCONF,
- "FAN_MARK_FILESYSTEM not supported in kernel?");
- return;
- }
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | %s | "
- "FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
- "failed", fd_notify, mark->name, fname);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+ FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname);
/*
* generate sequence of events
@@ -168,14 +157,9 @@ static void test_fanotify(unsigned int n)
*/
/* Ignore access events */
- if (fanotify_mark(fd_notify,
+ SAFE_FANOTIFY_MARK(fd_notify,
FAN_MARK_ADD | mark->flag | FAN_MARK_IGNORED_MASK,
- FAN_ACCESS, AT_FDCWD, fname) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s | "
- "FAN_MARK_IGNORED_MASK, FAN_ACCESS, AT_FDCWD, %s) "
- "failed", fd_notify, mark->name, fname);
- }
+ FAN_ACCESS, AT_FDCWD, fname);
fd = SAFE_OPEN(fname, O_RDWR);
event_set[tst_count] = FAN_OPEN;
@@ -218,15 +202,9 @@ static void test_fanotify(unsigned int n)
* Now ignore open & close events regardless of file
* modifications
*/
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag |
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag |
FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY,
- FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s | "
- "FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, "
- "FAN_OPEN | FAN_CLOSE, AT_FDCWD, %s) failed",
- fd_notify, mark->name, fname);
- }
+ FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
/* This event should be ignored */
fd = SAFE_OPEN(fname, O_RDWR);
@@ -247,15 +225,9 @@ static void test_fanotify(unsigned int n)
len += ret;
/* Now remove open and close from ignored mask */
- if (fanotify_mark(fd_notify,
+ SAFE_FANOTIFY_MARK(fd_notify,
FAN_MARK_REMOVE | mark->flag | FAN_MARK_IGNORED_MASK,
- FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_REMOVE | %s | "
- "FAN_MARK_IGNORED_MASK, FAN_OPEN | FAN_CLOSE, "
- "AT_FDCWD, %s) failed", fd_notify,
- mark->name, fname);
- }
+ FAN_OPEN | FAN_CLOSE, AT_FDCWD, fname);
SAFE_CLOSE(fd);
event_set[tst_count] = FAN_CLOSE_WRITE;
@@ -351,14 +323,9 @@ pass:
}
/* Remove mark to clear FAN_MARK_IGNORED_SURV_MODIFY */
- if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | mark->flag,
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | mark->flag,
FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
- AT_FDCWD, fname) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_REMOVE | %s, FAN_ACCESS | "
- "FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, %s) "
- "failed", fd_notify, mark->name, fname);
- }
+ AT_FDCWD, fname);
SAFE_CLOSE(fd_notify);
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify02.c b/testcases/kernel/syscalls/fanotify/fanotify02.c
index c578e0ae8..36c87648e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify02.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify02.c
@@ -46,16 +46,9 @@ void test01(void)
int tst_count = 0;
- if (fanotify_mark(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
- FAN_MODIFY | FAN_CLOSE | FAN_OPEN |
- FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD,
- ".") < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS | "
- "FAN_MODIFY | FAN_CLOSE | FAN_OPEN | "
- "FAN_EVENT_ON_CHILD | FAN_ONDIR, AT_FDCWD, '.') "
- "failed", fd_notify);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_ACCESS |
+ FAN_MODIFY | FAN_CLOSE | FAN_OPEN | FAN_EVENT_ON_CHILD |
+ FAN_ONDIR, AT_FDCWD, ".");
/*
* generate sequence of events
@@ -102,13 +95,8 @@ void test01(void)
/*
* now remove child mark
*/
- if (fanotify_mark(fd_notify, FAN_MARK_REMOVE,
- FAN_EVENT_ON_CHILD, AT_FDCWD, ".") < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK REMOVE, "
- "FAN_EVENT_ON_CHILD, AT_FDCWD, '.') failed",
- fd_notify);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE,
+ FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
/*
* Do something to verify events didn't get generated
diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
index ccb9a4799..5e5104fe5 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify03.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
@@ -216,22 +216,8 @@ static int setup_mark(unsigned int n)
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
for (; i < ARRAY_SIZE(files); i++) {
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
- tc->mask, AT_FDCWD, files[i]) < 0) {
- if (errno == EINVAL &&
- mark->flag == FAN_MARK_FILESYSTEM) {
- tst_res(TCONF,
- "FAN_MARK_FILESYSTEM not supported in "
- "kernel?");
- return -1;
- } else {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s, "
- "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
- "AT_FDCWD, %s) failed.",
- fd_notify, mark->name, fname);
- }
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+ tc->mask, AT_FDCWD, files[i]);
}
return 0;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
index 722ad5d41..a24e7f7c3 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify04.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
@@ -77,13 +77,8 @@ static void check_mark(char *file, unsigned long long flag, char *flagstr,
if (test_event)
test_event(file);
- if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | flag,
- FAN_OPEN, AT_FDCWD, file) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_REMOVE | %s, "
- "FAN_OPEN, AT_FDCWD, '%s') failed",
- fd_notify, flagstr, file);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | flag,
+ FAN_OPEN, AT_FDCWD, file);
}
}
@@ -191,29 +186,14 @@ void test01(void)
CHECK_MARK(sname, 0, 0, test_open_file);
/* Verify FAN_MARK_FLUSH destroys all inode marks */
- if (fanotify_mark(fd_notify, FAN_MARK_ADD,
- FAN_OPEN, AT_FDCWD, fname) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN, "
- "AT_FDCWD, '%s') failed", fd_notify, fname);
- }
- if (fanotify_mark(fd_notify, FAN_MARK_ADD,
- FAN_OPEN | FAN_ONDIR, AT_FDCWD, dir) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN | "
- "FAN_ONDIR, AT_FDCWD, '%s') failed", fd_notify,
- dir);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD, fname);
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR,
+ AT_FDCWD, dir);
open_file(fname);
verify_event(S_IFREG);
open_dir(dir);
verify_event(S_IFDIR);
- if (fanotify_mark(fd_notify, FAN_MARK_FLUSH,
- 0, AT_FDCWD, ".") < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_FLUSH, 0, "
- "AT_FDCWD, '.') failed", fd_notify);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_FLUSH, 0, AT_FDCWD, ".");
open_dir(dir);
verify_no_event();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
index e53cc333a..c13b9ad7b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify05.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
@@ -107,13 +107,8 @@ static void setup(void)
fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_NONBLOCK,
O_RDONLY);
- if (fanotify_mark(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
- AT_FDCWD, MOUNT_PATH) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_MOUNT | FAN_MARK_ADD, "
- "FAN_OPEN, AT_FDCWD, \".\") failed",
- fd_notify);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
+ AT_FDCWD, MOUNT_PATH);
}
static void cleanup(void)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
index 99e312a4f..ac4901f6f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify06.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
@@ -78,7 +78,6 @@ static struct tcase {
static void create_fanotify_groups(void)
{
unsigned int p, i;
- int ret;
for (p = 0; p < FANOTIFY_PRIORITIES; p++) {
for (i = 0; i < GROUPS_PER_PRIO; i++) {
@@ -87,32 +86,20 @@ static void create_fanotify_groups(void)
O_RDONLY);
/* Add mount mark for each group */
- ret = fanotify_mark(fd_notify[p][i],
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD | FAN_MARK_MOUNT,
FAN_MODIFY,
AT_FDCWD, fname);
- if (ret < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | "
- "FAN_MARK_MOUNT, FAN_MODIFY, AT_FDCWD,"
- " %s) failed", fd_notify[p][i], fname);
- }
+
/* Add ignore mark for groups with higher priority */
if (p == 0)
continue;
- ret = fanotify_mark(fd_notify[p][i],
+
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD |
FAN_MARK_IGNORED_MASK |
FAN_MARK_IGNORED_SURV_MODIFY,
FAN_MODIFY, AT_FDCWD, fname);
- if (ret < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | "
- "FAN_MARK_IGNORED_MASK | "
- "FAN_MARK_IGNORED_SURV_MODIFY, "
- "FAN_MODIFY, AT_FDCWD, %s) failed",
- fd_notify[p][i], fname);
- }
}
}
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index 533c880d0..4bf17661a 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -102,14 +102,7 @@ static int setup_instance(void)
int fd;
fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
-
- if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
- fname) < 0) {
- close(fd);
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, %s) failed.",
- fd, fname);
- }
+ SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);
return fd;
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
index 83210bc1c..9c9938619 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify09.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
@@ -99,7 +99,6 @@ static void create_fanotify_groups(struct tcase *tc)
{
struct fanotify_mark_type *mark = &tc->mark;
unsigned int i, onchild, ondir = tc->ondir;
- int ret;
for (i = 0; i < NUM_GROUPS; i++) {
fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
@@ -111,17 +110,11 @@ static void create_fanotify_groups(struct tcase *tc)
* but only the first group requests events on dir.
*/
onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
- ret = fanotify_mark(fd_notify[i],
+ SAFE_FANOTIFY_MARK(fd_notify[i],
FAN_MARK_ADD | mark->flag,
FAN_CLOSE_NOWRITE | onchild,
AT_FDCWD, tc->testdir);
- if (ret < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s, "
- "%x, AT_FDCWD, '%s') failed",
- fd_notify[i], mark->name,
- FAN_CLOSE_NOWRITE | ondir, tc->testdir);
- }
+
/*
* Add inode mark on parent for each group with MODIFY event,
* but only the first group requests events on child.
@@ -129,15 +122,9 @@ static void create_fanotify_groups(struct tcase *tc)
* setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
* flag.
*/
- ret = fanotify_mark(fd_notify[i], FAN_MARK_ADD,
+ SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
FAN_MODIFY | ondir | onchild,
AT_FDCWD, ".");
- if (ret < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD, "
- "%x, AT_FDCWD, '.') failed",
- fd_notify[i], FAN_MODIFY | ondir | onchild);
- }
}
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 82f8a7774..7794c2f04 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -288,7 +288,6 @@ static int create_fanotify_groups(unsigned int n)
struct fanotify_mark_type *mark, *ignore_mark;
unsigned int mark_ignored, mask;
unsigned int p, i;
- int ret;
mark = &fanotify_mark_types[tc->mark_type];
ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
@@ -317,32 +316,12 @@ static int create_fanotify_groups(unsigned int n)
* FAN_EVENT_ON_CHILD has no effect on filesystem/mount
* or inode mark on non-directory.
*/
- ret = fanotify_mark(fd_notify[p][i],
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD | mark->flag,
tc->expected_mask_without_ignore |
FAN_EVENT_ON_CHILD,
AT_FDCWD, tc->mark_path);
- if (ret < 0) {
- if (errno == EINVAL &&
- tc->expected_mask_without_ignore &
- FAN_OPEN_EXEC) {
- tst_res(TCONF,
- "FAN_OPEN_EXEC not supported "
- "by kernel?");
- return -1;
- } else if (errno == EINVAL &&
- tc->mark_type == FANOTIFY_FILESYSTEM) {
- tst_res(TCONF,
- "FAN_MARK_FILESYSTEM not "
- "supported in kernel?");
- return -1;
- }
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s,"
- "FAN_OPEN, AT_FDCWD, %s) failed",
- fd_notify[p][i], mark->name,
- tc->mark_path);
- }
+
/* Add ignore mark for groups with higher priority */
if (p == 0)
continue;
@@ -351,18 +330,9 @@ static int create_fanotify_groups(unsigned int n)
mark_ignored = FAN_MARK_IGNORED_MASK |
FAN_MARK_IGNORED_SURV_MODIFY;
add_mark:
- ret = fanotify_mark(fd_notify[p][i],
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
mask, AT_FDCWD, tc->ignore_path);
- if (ret < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s | %s, "
- "%x, AT_FDCWD, %s) failed",
- fd_notify[p][i], ignore_mark->name,
- mark_ignored ? "FAN_MARK_IGNORED_MASK | "
- "FAN_MARK_IGNORED_SURV_MODIFY" : "",
- mask, tc->ignore_path);
- }
/*
* If ignored mask is on a parent watching children,
diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
index 9e8606c72..56b7153f8 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify11.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
@@ -55,7 +55,6 @@ static unsigned int tcases[] = {
void test01(unsigned int i)
{
- int ret;
pthread_t p_id;
struct fanotify_event_metadata event;
int fd_notify;
@@ -76,10 +75,8 @@ void test01(unsigned int i)
tcases[i]);
}
- ret = fanotify_mark(fd_notify, FAN_MARK_ADD,
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
- if (ret != 0)
- tst_brk(TBROK, "fanotify_mark FAN_MARK_ADD fail ret=%d", ret);
SAFE_PTHREAD_CREATE(&p_id, NULL, thread_create_file, NULL);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
index dde1a3aea..1111ffb0b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify12.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
@@ -146,31 +146,15 @@ static int setup_mark(unsigned int n)
for (; i < ARRAY_SIZE(files); i++) {
/* Setup normal mark on object */
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
- tc->mask, AT_FDCWD, files[i]) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s, "
- "%llx, AT_FDCWD, %s) failed",
- fd_notify,
- mark->name,
- tc->mask,
- files[i]);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
+ tc->mask, AT_FDCWD, files[i]);
/* Setup ignore mark on object */
if (tc->ignore_mask) {
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag
| FAN_MARK_IGNORED_MASK,
tc->ignore_mask, AT_FDCWD,
- files[i]) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, "
- "FAN_MARK_ADD | %s "
- "| FAN_MARK_IGNORED_MASK, "
- "%llx, AT_FDCWD, %s) failed",
- fd_notify, mark->name,
- tc->ignore_mask, files[i]);
- }
+ files[i]);
}
}
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 39caea41e..2c1dc4624 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -116,28 +116,8 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
struct fanotify_mark_type *mark = &tc->mark;
for (i = 0; i < ARRAY_SIZE(objects); i++) {
- if (fanotify_mark(fd, FAN_MARK_ADD | mark->flag, tc->mask,
- AT_FDCWD, objects[i].path) == -1) {
- if (errno == EINVAL &&
- mark->flag & FAN_MARK_FILESYSTEM) {
- tst_res(TCONF,
- "FAN_MARK_FILESYSTEM not supported by "
- "kernel");
- return 1;
- } else if (errno == ENODEV &&
- !objects[i].fid.fsid.val[0] &&
- !objects[i].fid.fsid.val[1]) {
- tst_res(TCONF,
- "FAN_REPORT_FID not supported on "
- "filesystem type %s",
- tst_device->fs_type);
- return 1;
- }
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD, FAN_OPEN, "
- "AT_FDCWD, %s) failed",
- fanotify_fd, objects[i].path);
- }
+ SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD | mark->flag, tc->mask,
+ AT_FDCWD, objects[i].path);
/* Setup the expected mask for each generated event */
event_set[i].expected_mask = tc->mask;
@@ -294,13 +274,8 @@ static void do_setup(void)
* uninitialized connector->fsid cache. This mark remains for all test
* cases and is not expected to get any events (no writes in this test).
*/
- if (fanotify_mark(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
- FILE_PATH_ONE) == -1) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD, FAN_CLOSE_WRITE, "
- "AT_FDCWD, "FILE_PATH_ONE") failed",
- nofid_fd);
- }
+ SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
+ FILE_PATH_ONE);
/* Get the filesystem fsid and file handle for each created object */
get_object_stats();
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index c3fc4f8ab..6121ef211 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -85,17 +85,10 @@ static void do_test(unsigned int number)
tst_res(TINFO, "Test #%d: %s", number, tc->tname);
- if (fanotify_mark(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
+ SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_ADD | mark->flag, tc->mask |
FAN_CREATE | FAN_DELETE | FAN_MOVE |
FAN_MODIFY | FAN_ONDIR,
- AT_FDCWD, TEST_DIR) == -1) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark(%d, FAN_MARK_ADD | %s, "
- "FAN_CREATE | FAN_DELETE | FAN_MOVE | "
- "FAN_MODIFY | FAN_ONDIR | 0x%lx, "
- "AT_FDCWD, %s) failed",
- fanotify_fd, mark->name, tc->mask, TEST_DIR);
- }
+ AT_FDCWD, TEST_DIR);
/* Save the test root dir fid */
fanotify_save_fid(TEST_DIR, &root_fid);
@@ -187,13 +180,8 @@ static void do_test(unsigned int number)
/*
* Cleanup the mark
*/
- if (fanotify_mark(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
- AT_FDCWD, TEST_DIR) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_FLUSH | %s, 0,"
- "AT_FDCWD, '"TEST_DIR"') failed",
- fanotify_fd, mark->name);
- }
+ SAFE_FANOTIFY_MARK(fanotify_fd, FAN_MARK_FLUSH | mark->flag, 0,
+ AT_FDCWD, TEST_DIR);
/* Process each event in buffer */
for (i = 0, metadata = (struct fanotify_event_metadata *) events_buf;
diff --git a/testcases/kernel/syscalls/fanotify/fanotify16.c b/testcases/kernel/syscalls/fanotify/fanotify16.c
index e2a1509b0..86eb7e59b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify16.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify16.c
@@ -174,13 +174,8 @@ static void do_test(unsigned int number)
/*
* Watch dir modify events with name in filesystem/dir
*/
- if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
- AT_FDCWD, MOUNT_PATH) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
- "AT_FDCWD, '"MOUNT_PATH"') failed",
- fd_notify, mark->name, tc->mask);
- }
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag, tc->mask,
+ AT_FDCWD, MOUNT_PATH);
/* Save the mount root fid */
fanotify_save_fid(MOUNT_PATH, &root_fid);
@@ -195,14 +190,9 @@ static void do_test(unsigned int number)
/* Save the subdir fid */
fanotify_save_fid(dname1, &dir_fid);
- if (tc->sub_mask &&
- fanotify_mark(fd_notify, FAN_MARK_ADD | sub_mark->flag, tc->sub_mask,
- AT_FDCWD, dname1) < 0) {
- tst_brk(TBROK | TERRNO,
- "fanotify_mark (%d, FAN_MARK_ADD | %s, 0x%lx, "
- "AT_FDCWD, '%s') failed",
- fd_notify, sub_mark->name, tc->sub_mask, dname1);
- }
+ if (tc->sub_mask)
+ SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | sub_mark->flag,
+ tc->sub_mask, AT_FDCWD, dname1);
event_set[tst_count].mask = FAN_CREATE | FAN_ONDIR;
event_set[tst_count].fid = &root_fid;
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
` (3 preceding siblings ...)
2020-11-26 21:41 ` [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-27 14:22 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 6/6] fanotify: Add a pedantic check for return value Petr Vorel
` (2 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
in safe_fanotify_init()
That require to move the definition after flags.
Also use tst_res_()/tst_brk_() instead of tst_res()/tst_brk() in
safe_fanotify_init() (synchronize with safe_fanotify_mark()).
Make use of this simplification in fanotify15.c.
Suggested-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Changes v3->v4:
* use tst_res_() and tst_brk_() for safe_*() functions
testcases/kernel/syscalls/fanotify/fanotify.h | 64 +++++++++++--------
.../kernel/syscalls/fanotify/fanotify01.c | 13 +---
.../kernel/syscalls/fanotify/fanotify13.c | 12 +---
.../kernel/syscalls/fanotify/fanotify15.c | 9 +--
4 files changed, 41 insertions(+), 57 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 2a5280636..4b725f334 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -38,32 +38,6 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
#endif /* HAVE_SYS_FANOTIFY_H */
-int safe_fanotify_init(const char *file, const int lineno,
- unsigned int flags, unsigned int event_f_flags)
-{
- int rval;
-
-#ifdef HAVE_SYS_FANOTIFY_H
- rval = fanotify_init(flags, event_f_flags);
-
- if (rval == -1) {
- if (errno == ENOSYS) {
- tst_brk(TCONF,
- "fanotify is not configured in this kernel.");
- }
- tst_brk(TBROK | TERRNO,
- "%s:%d: fanotify_init() failed", file, lineno);
- }
-#else
- tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
-#endif /* HAVE_SYS_FANOTIFY_H */
-
- return rval;
-}
-
-#define SAFE_FANOTIFY_INIT(fan, mode) \
- safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
-
#ifndef FAN_REPORT_TID
#define FAN_REPORT_TID 0x00000100
#endif
@@ -190,6 +164,44 @@ struct fanotify_event_info_fid {
#define MAX_HANDLE_SZ 128
#endif
+int safe_fanotify_init(const char *file, const int lineno,
+ unsigned int flags, unsigned int event_f_flags)
+{
+ int rval;
+
+#ifdef HAVE_SYS_FANOTIFY_H
+ rval = fanotify_init(flags, event_f_flags);
+
+ if (rval == -1) {
+ if (errno == ENOSYS) {
+ tst_brk_(file, lineno, TCONF,
+ "fanotify not configured in kernel");
+ }
+
+ if (errno == EINVAL) {
+ if (flags & FAN_REPORT_FID) {
+ tst_brk_(file, lineno, TCONF,
+ "FAN_REPORT_FID not supported in kernel?");
+ }
+
+ if (flags & FAN_REPORT_NAME) {
+ tst_brk_(file, lineno, TCONF,
+ "FAN_REPORT_NAME not supported in kernel?");
+ }
+ }
+
+ tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_init() failed");
+ }
+#else
+ tst_brk_(file, lineno, TCONF, "Header <sys/fanotify.h> is not present");
+#endif /* HAVE_SYS_FANOTIFY_H */
+
+ return rval;
+}
+
+#define SAFE_FANOTIFY_INIT(fan, mode) \
+ safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
+
static inline int safe_fanotify_mark(const char *file, const int lineno,
int fd, unsigned int flags, uint64_t mask,
int dfd, const char *pathname)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify01.c b/testcases/kernel/syscalls/fanotify/fanotify01.c
index 170428eba..cd571e002 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify01.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify01.c
@@ -95,18 +95,7 @@ static void test_fanotify(unsigned int n)
return;
}
- fd_notify = fanotify_init(tc->init_flags, O_RDONLY);
- if (fd_notify < 0) {
- if (errno == EINVAL &&
- (tc->init_flags & FAN_REPORT_FID)) {
- tst_res(TCONF,
- "FAN_REPORT_FID not supported in kernel?");
- return;
- }
- tst_brk(TBROK | TERRNO,
- "fanotify_init (0x%x, O_RDONLY) "
- "failed", tc->init_flags);
- }
+ fd_notify = SAFE_FANOTIFY_INIT(tc->init_flags, O_RDONLY);
SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN, AT_FDCWD, fname);
diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 2c1dc4624..3d5cfc13b 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -142,17 +142,7 @@ static void do_test(unsigned int number)
"Test #%d: FAN_REPORT_FID with mark flag: %s",
number, mark->name);
- fanotify_fd = fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
- if (fanotify_fd == -1) {
- if (errno == EINVAL) {
- tst_res(TCONF,
- "FAN_REPORT_FID not supported by kernel");
- return;
- }
- tst_brk(TBROK | TERRNO,
- "fanotify_init(FAN_CLASS_NOTIF | FAN_REPORT_FID, "
- "O_RDONLY) failed");
- }
+ fanotify_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
/*
* Place marks on a set of objects and setup the expected masks
diff --git a/testcases/kernel/syscalls/fanotify/fanotify15.c b/testcases/kernel/syscalls/fanotify/fanotify15.c
index 6121ef211..8bf8f2045 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify15.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify15.c
@@ -277,14 +277,7 @@ static void do_setup(void)
fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
SAFE_CLOSE(fd);
- fanotify_fd = fanotify_init(FAN_REPORT_FID, O_RDONLY);
- if (fanotify_fd == -1) {
- if (errno == EINVAL)
- tst_brk(TCONF,
- "FAN_REPORT_FID not supported in kernel");
- tst_brk(TBROK | TERRNO,
- "fanotify_init(FAN_REPORT_FID, O_RDONLY) failed");
- }
+ fanotify_fd = SAFE_FANOTIFY_INIT(FAN_REPORT_FID, O_RDONLY);
SAFE_MKDIR(TEST_DIR, 0755);
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 6/6] fanotify: Add a pedantic check for return value
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
` (4 preceding siblings ...)
2020-11-26 21:41 ` [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support Petr Vorel
@ 2020-11-26 21:41 ` Petr Vorel
2020-11-26 21:47 ` [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-11-27 15:36 ` Amir Goldstein
7 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:41 UTC (permalink / raw)
To: ltp
for fanotify_init() in safe_fanotify_init()
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
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 4b725f334..e75b27754 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -192,6 +192,11 @@ int safe_fanotify_init(const char *file, const int lineno,
tst_brk_(file, lineno, TBROK | TERRNO, "fanotify_init() failed");
}
+
+ if (rval < -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "invalid fanotify_init() return %d", rval);
+ }
#else
tst_brk_(file, lineno, TCONF, "Header <sys/fanotify.h> is not present");
#endif /* HAVE_SYS_FANOTIFY_H */
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
` (5 preceding siblings ...)
2020-11-26 21:41 ` [LTP] [PATCH v4 6/6] fanotify: Add a pedantic check for return value Petr Vorel
@ 2020-11-26 21:47 ` Petr Vorel
2020-11-27 15:36 ` Amir Goldstein
7 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2020-11-26 21:47 UTC (permalink / raw)
To: ltp
BTW:
FYI this is the first pachset which broke CentOS 6
(kernel: 2.6.32-696, glibc: 2.12).
But we've just agreed to not support it...
In file included from fanotify01.c:21:
fanotify.h: In function ?require_fanotify_access_permissions_supported_by_kernel?:
fanotify.h:301: warning: implicit declaration of function ?SAFE_FANOTIFY_INIT?
fanotify.h:301: error: ?FAN_CLASS_CONTENT? undeclared (first use in this function)
fanotify.h:301: error: (Each undeclared identifier is reported only once
fanotify.h:301: error: for each function it appears in.)
fanotify.h:303: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h:303: error: ?FAN_ACCESS_PERM? undeclared (first use in this function)
fanotify.h: In function ?fanotify_exec_events_supported_by_kernel?:
fanotify.h:321: error: ?FAN_CLASS_CONTENT? undeclared (first use in this function)
fanotify.h:323: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h: In function ?fanotify_fan_report_fid_supported_on_fs?:
fanotify.h:342: error: ?FAN_CLASS_NOTIF? undeclared (first use in this function)
fanotify.h:344: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_ACCESS? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_MODIFY? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_CLOSE? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_OPEN? undeclared (first use in this function)
make: *** [fanotify01] Error 1
make: *** Waiting for unfinished jobs....
In file included from fanotify02.c:21:
fanotify.h: In function ?require_fanotify_access_permissions_supported_by_kernel?:
fanotify.h:301: warning: implicit declaration of function ?SAFE_FANOTIFY_INIT?
fanotify.h:301: error: ?FAN_CLASS_CONTENT? undeclared (first use in this function)
fanotify.h:301: error: (Each undeclared identifier is reported only once
fanotify.h:301: error: for each function it appears in.)
fanotify.h:303: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h:303: error: ?FAN_ACCESS_PERM? undeclared (first use in this function)
fanotify.h: In function ?fanotify_exec_events_supported_by_kernel?:
fanotify.h:321: error: ?FAN_CLASS_CONTENT? undeclared (first use in this function)
fanotify.h:323: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h: In function ?fanotify_fan_report_fid_supported_on_fs?:
fanotify.h:342: error: ?FAN_CLASS_NOTIF? undeclared (first use in this function)
fanotify.h:344: error: ?FAN_MARK_ADD? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_ACCESS? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_MODIFY? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_CLOSE? undeclared (first use in this function)
fanotify.h:345: error: ?FAN_OPEN? undeclared (first use in this function)
make: *** [fanotify02] Error 1
Kind regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup()
2020-11-26 21:41 ` [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup() Petr Vorel
@ 2020-11-27 13:20 ` Amir Goldstein
0 siblings, 0 replies; 14+ messages in thread
From: Amir Goldstein @ 2020-11-27 13:20 UTC (permalink / raw)
To: ltp
On Thu, Nov 26, 2020 at 11:41 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Create 2 helpers which are used in various tests in setup() to check for
> FAN_ACCESS_PERM enablement (caused by disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS)
> or FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM support in kernel.
>
> That helps to further code simplification in next commit.
>
> Due this change test with FAN_OPEN_EXEC_PERM in fanotify03.c no longer
> needs to be a first test.
>
> Also rename variable s/support_exec_events/exec_events_unsupported/
> for readability.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
You may add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Just one nit I wrote it in review and you must have missed it.
There is nothing related to "exec" in the helper.
It should be called fanotify_events_supported_by_kernel().
Thanks,
Amir.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs
2020-11-26 21:41 ` [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs Petr Vorel
@ 2020-11-27 13:47 ` Amir Goldstein
2020-11-27 14:26 ` Petr Vorel
0 siblings, 1 reply; 14+ messages in thread
From: Amir Goldstein @ 2020-11-27 13:47 UTC (permalink / raw)
To: ltp
On Thu, Nov 26, 2020 at 11:41 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> This is related to kernel fix
> a8b13aa20afb ("fanotify: enable FAN_REPORT_FID init flag")
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
Just a minor nit below.
you may add:
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
As far as I am concerned, you do not need to re-post for the nits
if Cyril is going to fix my nit on merge (or even if he doesn't)
> ---
> New in v4.
> Maybe it'd deserve better commit message.
>
> There might be even more cleanup: not sure if nofid_fd in fanotify13.c
> is required. According to the description is probably required:
You're right, It is required.
>
> static void do_setup(void)
> {
> require_fanotify_fan_report_fid_supported_on_fs(MOUNT_PATH);
>
> nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
>
> /* Create file and directory objects for testing */
> create_objects();
>
> /*
> * Create a mark on first inode without FAN_REPORT_FID, to test
> * uninitialized connector->fsid cache. This mark remains for all test
> * cases and is not expected to get any events (no writes in this test).
> */
> SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
> FILE_PATH_ONE);
>
> /* Get the filesystem fsid and file handle for each created object */
> get_object_stats();
>
> testcases/kernel/syscalls/fanotify/fanotify.h | 31 +++++++++++++++++++
> .../kernel/syscalls/fanotify/fanotify01.c | 9 +++++-
> .../kernel/syscalls/fanotify/fanotify13.c | 4 ++-
> .../kernel/syscalls/fanotify/fanotify15.c | 6 ++--
> .../kernel/syscalls/fanotify/fanotify16.c | 6 +---
> 5 files changed, 45 insertions(+), 11 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 413034336..c690b82d3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -283,4 +283,35 @@ static inline int fanotify_exec_events_supported_by_kernel(uint64_t mask)
> return rval;
> }
>
> +static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
> +{
> + int fd;
> + int rval = 0;
> +
> + fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
> +
> + if (fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> + FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> + AT_FDCWD, fname) < 0) {
All those flags are not really needed for the test.
This minimal arg list would have been enough:
fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname)
Thanks,
Amir.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support
2020-11-26 21:41 ` [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support Petr Vorel
@ 2020-11-27 14:22 ` Amir Goldstein
0 siblings, 0 replies; 14+ messages in thread
From: Amir Goldstein @ 2020-11-27 14:22 UTC (permalink / raw)
To: ltp
On Thu, Nov 26, 2020 at 11:41 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> in safe_fanotify_init()
>
> That require to move the definition after flags.
>
> Also use tst_res_()/tst_brk_() instead of tst_res()/tst_brk() in
> safe_fanotify_init() (synchronize with safe_fanotify_mark()).
>
> Make use of this simplification in fanotify15.c.
>
> Suggested-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes v3->v4:
> * use tst_res_() and tst_brk_() for safe_*() functions
>
> testcases/kernel/syscalls/fanotify/fanotify.h | 64 +++++++++++--------
> .../kernel/syscalls/fanotify/fanotify01.c | 13 +---
> .../kernel/syscalls/fanotify/fanotify13.c | 12 +---
> .../kernel/syscalls/fanotify/fanotify15.c | 9 +--
> 4 files changed, 41 insertions(+), 57 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 2a5280636..4b725f334 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -38,32 +38,6 @@ static long fanotify_mark(int fd, unsigned int flags, uint64_t mask,
>
> #endif /* HAVE_SYS_FANOTIFY_H */
>
> -int safe_fanotify_init(const char *file, const int lineno,
> - unsigned int flags, unsigned int event_f_flags)
> -{
> - int rval;
> -
> -#ifdef HAVE_SYS_FANOTIFY_H
> - rval = fanotify_init(flags, event_f_flags);
> -
> - if (rval == -1) {
> - if (errno == ENOSYS) {
> - tst_brk(TCONF,
> - "fanotify is not configured in this kernel.");
> - }
> - tst_brk(TBROK | TERRNO,
> - "%s:%d: fanotify_init() failed", file, lineno);
> - }
> -#else
> - tst_brk(TCONF, "Header <sys/fanotify.h> is not present");
> -#endif /* HAVE_SYS_FANOTIFY_H */
> -
> - return rval;
> -}
> -
> -#define SAFE_FANOTIFY_INIT(fan, mode) \
> - safe_fanotify_init(__FILE__, __LINE__, (fan), (mode))
> -
> #ifndef FAN_REPORT_TID
> #define FAN_REPORT_TID 0x00000100
> #endif
> @@ -190,6 +164,44 @@ struct fanotify_event_info_fid {
> #define MAX_HANDLE_SZ 128
> #endif
>
> +int safe_fanotify_init(const char *file, const int lineno,
> + unsigned int flags, unsigned int event_f_flags)
> +{
> + int rval;
> +
> +#ifdef HAVE_SYS_FANOTIFY_H
> + rval = fanotify_init(flags, event_f_flags);
> +
> + if (rval == -1) {
> + if (errno == ENOSYS) {
> + tst_brk_(file, lineno, TCONF,
> + "fanotify not configured in kernel");
> + }
> +
> + if (errno == EINVAL) {
> + if (flags & FAN_REPORT_FID) {
> + tst_brk_(file, lineno, TCONF,
> + "FAN_REPORT_FID not supported in kernel?");
> + }
Unless I am missing something, this would make
fanotify_fan_report_fid_supported_on_fs()
break on old kernel in the start of fanotify01. That was not the intention...
The FAN_REPORT_FID test case in fanotify01 should be skipped if either
kernel has no support for FAN_REPORT_FID (checked on fanotify_init) or
filesystem has no support for FAN_REPORT_FID (checked on fanotify_mark).
I guess you can leave SAFE_FANOTIFY_INIT as is and change the helper
fanotify_fan_report_fid_supported() to return a different value for
unsupported by kernel
and unsupported by fs and then pass that value of
fan_report_fid_unsupported to another
helper to decide if test case should be skipped and print the proper
TCONF message.
> +
> + if (flags & FAN_REPORT_NAME) {
> + tst_brk_(file, lineno, TCONF,
> + "FAN_REPORT_NAME not supported in kernel?");
> + }
This check should be before the check for FAN_REPORT_FID, because
a kernel that does not support FAN_REPORT_FID also does not support
FAN_REPORT_NAME, but not the other way around.
I think that also solves the problem you mentioned in the cover letter
about fanotify16, but if not you can leave that problem to me.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs
2020-11-27 13:47 ` Amir Goldstein
@ 2020-11-27 14:26 ` Petr Vorel
0 siblings, 0 replies; 14+ messages in thread
From: Petr Vorel @ 2020-11-27 14:26 UTC (permalink / raw)
To: ltp
Hi Amir,
> Just a minor nit below.
> you may add:
> Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Sure. Thanks for your review!
> As far as I am concerned, you do not need to re-post for the nits
> if Cyril is going to fix my nit on merge (or even if he doesn't)
Sure, I also hope that nothing controversial appear after 4 reviews.
...
> > There might be even more cleanup: not sure if nofid_fd in fanotify13.c
> > is required. According to the description is probably required:
> You're right, It is required.
Thanks for confirmation!
...
> > --- a/testcases/kernel/syscalls/fanotify/fanotify.h
...
> > +static inline int fanotify_fan_report_fid_supported_on_fs(const char *fname)
> > +{
> > + int fd;
> > + int rval = 0;
> > +
> > + fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_REPORT_FID, O_RDONLY);
> > +
> > + if (fanotify_mark(fd, FAN_MARK_ADD | FAN_MARK_FILESYSTEM,
> > + FAN_ACCESS | FAN_MODIFY | FAN_CLOSE | FAN_OPEN,
> > + AT_FDCWD, fname) < 0) {
> All those flags are not really needed for the test.
> This minimal arg list would have been enough:
> fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS, AT_FDCWD, fname)
Ack, thanks!
> Thanks,
> Amir.
Kind regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro
2020-11-26 21:41 ` [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
@ 2020-11-27 15:17 ` Amir Goldstein
0 siblings, 0 replies; 14+ messages in thread
From: Amir Goldstein @ 2020-11-27 15:17 UTC (permalink / raw)
To: ltp
On Thu, Nov 26, 2020 at 11:41 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> and function and use it in all tests which use fanotify_mark().
>
> Motivation was not only to simplify the code but also check for
> unsupported filesystems which was missing least for exfat on fanotify16,
> because filesystem can be set via LTP_DEV_FS_TYPE environment variable
> on tests which don't tests all filesystems.
>
> Previous commit added check for FAN_ACCESS_PERM enablement (caused by
> disabled CONFIG_FANOTIFY_ACCESS_PERMISSIONS) or FAN_OPEN_EXEC and
> FAN_OPEN_EXEC_PERM support in kernel, which are tested in setup
> functions for relevant tests, thus only FAN_MARK_FILESYSTEM is needed to
> check in safe_fanotify_mark().
>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Changes v3->v4:
> * use tst_res_() and tst_brk_() for safe_*() functions
>
> testcases/kernel/syscalls/fanotify/fanotify.h | 34 ++++++++++++
> .../kernel/syscalls/fanotify/fanotify01.c | 53 ++++---------------
> .../kernel/syscalls/fanotify/fanotify02.c | 22 ++------
> .../kernel/syscalls/fanotify/fanotify03.c | 18 +------
> .../kernel/syscalls/fanotify/fanotify04.c | 32 +++--------
> .../kernel/syscalls/fanotify/fanotify05.c | 9 +---
> .../kernel/syscalls/fanotify/fanotify06.c | 21 ++------
> .../kernel/syscalls/fanotify/fanotify07.c | 9 +---
> .../kernel/syscalls/fanotify/fanotify09.c | 19 ++-----
> .../kernel/syscalls/fanotify/fanotify10.c | 36 ++-----------
> .../kernel/syscalls/fanotify/fanotify11.c | 5 +-
> .../kernel/syscalls/fanotify/fanotify12.c | 24 ++-------
> .../kernel/syscalls/fanotify/fanotify13.c | 33 ++----------
> .../kernel/syscalls/fanotify/fanotify15.c | 20 ++-----
> .../kernel/syscalls/fanotify/fanotify16.c | 20 ++-----
> 15 files changed, 88 insertions(+), 267 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index c690b82d3..2a5280636 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -190,6 +190,40 @@ struct fanotify_event_info_fid {
> #define MAX_HANDLE_SZ 128
> #endif
>
> +static inline int safe_fanotify_mark(const char *file, const int lineno,
> + int fd, unsigned int flags, uint64_t mask,
> + int dfd, const char *pathname)
> +{
> + int rval;
> +
> + rval = fanotify_mark(fd, flags, mask, dfd, pathname);
> +
> + if (rval == -1) {
> + if (errno == ENODEV || errno == EOPNOTSUPP || errno == EXDEV) {
> + tst_brk_(file, lineno, TCONF,
> + "some FAN_REPORT_* flag not supported on %s filesystem",
> + tst_device->fs_type);
> + }
This is not needed here.
It was already tested by fanotify_fan_report_fid_supported_on_fs()
> +
> + if (errno == EINVAL && (flags & FAN_MARK_FILESYSTEM)) {
> + tst_brk_(file, lineno, TCONF,
> + "FAN_MARK_FILESYSTEM not supported in kernel?");
> + }
This turns test_res to test_brk in all the tests where FAN_MARK_FILESYSTEM
test cases should be skipped. What am I missing?
I guess we need another helper to check fanotify_filesystem_mark_support.
For example, in fanotify03:
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);
Note that there is no need for fanotify_mark_supported_by_kernel() for tests
where all test cases use FAN_REPORT_FID (fanotify15, fanotify16),
because all kernels that support FAN_REPORT_FID support FAN_MARK_FILESYSTEM.
[...]
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify03.c b/testcases/kernel/syscalls/fanotify/fanotify03.c
> index ccb9a4799..5e5104fe5 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify03.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify03.c
> @@ -216,22 +216,8 @@ static int setup_mark(unsigned int n)
> fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
>
> for (; i < ARRAY_SIZE(files); i++) {
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> - tc->mask, AT_FDCWD, files[i]) < 0) {
> - if (errno == EINVAL &&
> - mark->flag == FAN_MARK_FILESYSTEM) {
> - tst_res(TCONF,
> - "FAN_MARK_FILESYSTEM not supported in "
> - "kernel?");
All these should be removed by a previous patch that adds
fanotify_mark_supported_by_kernel() and skips test cases in advance.
> - return -1;
> - } else {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> - "FAN_ACCESS_PERM | FAN_OPEN_PERM, "
> - "AT_FDCWD, %s) failed.",
> - fd_notify, mark->name, fname);
> - }
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
> + tc->mask, AT_FDCWD, files[i]);
> }
>
> return 0;
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify04.c b/testcases/kernel/syscalls/fanotify/fanotify04.c
> index 722ad5d41..a24e7f7c3 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify04.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify04.c
> @@ -77,13 +77,8 @@ static void check_mark(char *file, unsigned long long flag, char *flagstr,
> if (test_event)
> test_event(file);
>
> - if (fanotify_mark(fd_notify, FAN_MARK_REMOVE | flag,
> - FAN_OPEN, AT_FDCWD, file) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_REMOVE | %s, "
> - "FAN_OPEN, AT_FDCWD, '%s') failed",
> - fd_notify, flagstr, file);
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_REMOVE | flag,
> + FAN_OPEN, AT_FDCWD, file);
> }
> }
>
> @@ -191,29 +186,14 @@ void test01(void)
> CHECK_MARK(sname, 0, 0, test_open_file);
>
> /* Verify FAN_MARK_FLUSH destroys all inode marks */
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD,
> - FAN_OPEN, AT_FDCWD, fname) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN, "
> - "AT_FDCWD, '%s') failed", fd_notify, fname);
> - }
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD,
> - FAN_OPEN | FAN_ONDIR, AT_FDCWD, dir) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_ADD, FAN_OPEN | "
> - "FAN_ONDIR, AT_FDCWD, '%s') failed", fd_notify,
> - dir);
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN, AT_FDCWD, fname);
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD, FAN_OPEN | FAN_ONDIR,
> + AT_FDCWD, dir);
> open_file(fname);
> verify_event(S_IFREG);
> open_dir(dir);
> verify_event(S_IFDIR);
> - if (fanotify_mark(fd_notify, FAN_MARK_FLUSH,
> - 0, AT_FDCWD, ".") < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_FLUSH, 0, "
> - "AT_FDCWD, '.') failed", fd_notify);
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_FLUSH, 0, AT_FDCWD, ".");
>
> open_dir(dir);
> verify_no_event();
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify05.c b/testcases/kernel/syscalls/fanotify/fanotify05.c
> index e53cc333a..c13b9ad7b 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify05.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify05.c
> @@ -107,13 +107,8 @@ static void setup(void)
> fd_notify = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | FAN_NONBLOCK,
> O_RDONLY);
>
> - if (fanotify_mark(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
> - AT_FDCWD, MOUNT_PATH) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_MOUNT | FAN_MARK_ADD, "
> - "FAN_OPEN, AT_FDCWD, \".\") failed",
> - fd_notify);
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_MOUNT | FAN_MARK_ADD, FAN_OPEN,
> + AT_FDCWD, MOUNT_PATH);
> }
>
> static void cleanup(void)
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify06.c b/testcases/kernel/syscalls/fanotify/fanotify06.c
> index 99e312a4f..ac4901f6f 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify06.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify06.c
> @@ -78,7 +78,6 @@ static struct tcase {
> static void create_fanotify_groups(void)
> {
> unsigned int p, i;
> - int ret;
>
> for (p = 0; p < FANOTIFY_PRIORITIES; p++) {
> for (i = 0; i < GROUPS_PER_PRIO; i++) {
> @@ -87,32 +86,20 @@ static void create_fanotify_groups(void)
> O_RDONLY);
>
> /* Add mount mark for each group */
> - ret = fanotify_mark(fd_notify[p][i],
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD | FAN_MARK_MOUNT,
> FAN_MODIFY,
> AT_FDCWD, fname);
> - if (ret < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | "
> - "FAN_MARK_MOUNT, FAN_MODIFY, AT_FDCWD,"
> - " %s) failed", fd_notify[p][i], fname);
> - }
> +
> /* Add ignore mark for groups with higher priority */
> if (p == 0)
> continue;
> - ret = fanotify_mark(fd_notify[p][i],
> +
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD |
> FAN_MARK_IGNORED_MASK |
> FAN_MARK_IGNORED_SURV_MODIFY,
> FAN_MODIFY, AT_FDCWD, fname);
> - if (ret < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | "
> - "FAN_MARK_IGNORED_MASK | "
> - "FAN_MARK_IGNORED_SURV_MODIFY, "
> - "FAN_MODIFY, AT_FDCWD, %s) failed",
> - fd_notify[p][i], fname);
> - }
> }
> }
> }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
> index 533c880d0..4bf17661a 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> @@ -102,14 +102,7 @@ static int setup_instance(void)
> int fd;
>
> fd = SAFE_FANOTIFY_INIT(FAN_CLASS_CONTENT, O_RDONLY);
> -
> - if (fanotify_mark(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD,
> - fname) < 0) {
> - close(fd);
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, %s) failed.",
> - fd, fname);
> - }
> + SAFE_FANOTIFY_MARK(fd, FAN_MARK_ADD, FAN_ACCESS_PERM, AT_FDCWD, fname);
>
> return fd;
> }
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c
> index 83210bc1c..9c9938619 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify09.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c
> @@ -99,7 +99,6 @@ static void create_fanotify_groups(struct tcase *tc)
> {
> struct fanotify_mark_type *mark = &tc->mark;
> unsigned int i, onchild, ondir = tc->ondir;
> - int ret;
>
> for (i = 0; i < NUM_GROUPS; i++) {
> fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF |
> @@ -111,17 +110,11 @@ static void create_fanotify_groups(struct tcase *tc)
> * but only the first group requests events on dir.
> */
> onchild = (i == 0) ? FAN_EVENT_ON_CHILD | ondir : 0;
> - ret = fanotify_mark(fd_notify[i],
> + SAFE_FANOTIFY_MARK(fd_notify[i],
> FAN_MARK_ADD | mark->flag,
> FAN_CLOSE_NOWRITE | onchild,
> AT_FDCWD, tc->testdir);
> - if (ret < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> - "%x, AT_FDCWD, '%s') failed",
> - fd_notify[i], mark->name,
> - FAN_CLOSE_NOWRITE | ondir, tc->testdir);
> - }
> +
> /*
> * Add inode mark on parent for each group with MODIFY event,
> * but only the first group requests events on child.
> @@ -129,15 +122,9 @@ static void create_fanotify_groups(struct tcase *tc)
> * setting the DCACHE_FSNOTIFY_PARENT_WATCHED dentry
> * flag.
> */
> - ret = fanotify_mark(fd_notify[i], FAN_MARK_ADD,
> + SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD,
> FAN_MODIFY | ondir | onchild,
> AT_FDCWD, ".");
> - if (ret < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD, "
> - "%x, AT_FDCWD, '.') failed",
> - fd_notify[i], FAN_MODIFY | ondir | onchild);
> - }
> }
> }
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index 82f8a7774..7794c2f04 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -288,7 +288,6 @@ static int create_fanotify_groups(unsigned int n)
> struct fanotify_mark_type *mark, *ignore_mark;
> unsigned int mark_ignored, mask;
> unsigned int p, i;
> - int ret;
>
> mark = &fanotify_mark_types[tc->mark_type];
> ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
> @@ -317,32 +316,12 @@ static int create_fanotify_groups(unsigned int n)
> * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
> * or inode mark on non-directory.
> */
> - ret = fanotify_mark(fd_notify[p][i],
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD | mark->flag,
> tc->expected_mask_without_ignore |
> FAN_EVENT_ON_CHILD,
> AT_FDCWD, tc->mark_path);
> - if (ret < 0) {
> - if (errno == EINVAL &&
> - tc->expected_mask_without_ignore &
> - FAN_OPEN_EXEC) {
> - tst_res(TCONF,
> - "FAN_OPEN_EXEC not supported "
> - "by kernel?");
> - return -1;
This should not have been here.
Should have been removed by exec_events_unsupported patch just like you did
with fanotify03 and fanotify07.
> - } else if (errno == EINVAL &&
> - tc->mark_type == FANOTIFY_FILESYSTEM) {
> - tst_res(TCONF,
> - "FAN_MARK_FILESYSTEM not "
> - "supported in kernel?");
> - return -1;
> - }
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | %s,"
> - "FAN_OPEN, AT_FDCWD, %s) failed",
> - fd_notify[p][i], mark->name,
> - tc->mark_path);
> - }
> +
> /* Add ignore mark for groups with higher priority */
> if (p == 0)
> continue;
> @@ -351,18 +330,9 @@ static int create_fanotify_groups(unsigned int n)
> mark_ignored = FAN_MARK_IGNORED_MASK |
> FAN_MARK_IGNORED_SURV_MODIFY;
> add_mark:
> - ret = fanotify_mark(fd_notify[p][i],
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
> mask, AT_FDCWD, tc->ignore_path);
> - if (ret < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | %s | %s, "
> - "%x, AT_FDCWD, %s) failed",
> - fd_notify[p][i], ignore_mark->name,
> - mark_ignored ? "FAN_MARK_IGNORED_MASK | "
> - "FAN_MARK_IGNORED_SURV_MODIFY" : "",
> - mask, tc->ignore_path);
> - }
>
> /*
> * If ignored mask is on a parent watching children,
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify11.c b/testcases/kernel/syscalls/fanotify/fanotify11.c
> index 9e8606c72..56b7153f8 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify11.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify11.c
> @@ -55,7 +55,6 @@ static unsigned int tcases[] = {
>
> void test01(unsigned int i)
> {
> - int ret;
> pthread_t p_id;
> struct fanotify_event_metadata event;
> int fd_notify;
> @@ -76,10 +75,8 @@ void test01(unsigned int i)
> tcases[i]);
> }
>
> - ret = fanotify_mark(fd_notify, FAN_MARK_ADD,
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD,
> FAN_ALL_EVENTS | FAN_EVENT_ON_CHILD, AT_FDCWD, ".");
> - if (ret != 0)
> - tst_brk(TBROK, "fanotify_mark FAN_MARK_ADD fail ret=%d", ret);
>
> SAFE_PTHREAD_CREATE(&p_id, NULL, thread_create_file, NULL);
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify12.c b/testcases/kernel/syscalls/fanotify/fanotify12.c
> index dde1a3aea..1111ffb0b 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify12.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify12.c
> @@ -146,31 +146,15 @@ static int setup_mark(unsigned int n)
>
> for (; i < ARRAY_SIZE(files); i++) {
> /* Setup normal mark on object */
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag,
> - tc->mask, AT_FDCWD, files[i]) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark(%d, FAN_MARK_ADD | %s, "
> - "%llx, AT_FDCWD, %s) failed",
> - fd_notify,
> - mark->name,
> - tc->mask,
> - files[i]);
> - }
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag,
> + tc->mask, AT_FDCWD, files[i]);
>
> /* Setup ignore mark on object */
> if (tc->ignore_mask) {
> - if (fanotify_mark(fd_notify, FAN_MARK_ADD | mark->flag
> + SAFE_FANOTIFY_MARK(fd_notify, FAN_MARK_ADD | mark->flag
> | FAN_MARK_IGNORED_MASK,
> tc->ignore_mask, AT_FDCWD,
> - files[i]) < 0) {
> - tst_brk(TBROK | TERRNO,
> - "fanotify_mark (%d, "
> - "FAN_MARK_ADD | %s "
> - "| FAN_MARK_IGNORED_MASK, "
> - "%llx, AT_FDCWD, %s) failed",
> - fd_notify, mark->name,
> - tc->ignore_mask, files[i]);
> - }
> + files[i]);
> }
> }
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index 39caea41e..2c1dc4624 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -116,28 +116,8 @@ static int setup_marks(unsigned int fd, struct test_case_t *tc)
> struct fanotify_mark_type *mark = &tc->mark;
>
> for (i = 0; i < ARRAY_SIZE(objects); i++) {
> - if (fanotify_mark(fd, FAN_MARK_ADD | mark->flag, tc->mask,
> - AT_FDCWD, objects[i].path) == -1) {
> - if (errno == EINVAL &&
> - mark->flag & FAN_MARK_FILESYSTEM) {
> - tst_res(TCONF,
> - "FAN_MARK_FILESYSTEM not supported by "
> - "kernel");
> - return 1;
> - } else if (errno == ENODEV &&
> - !objects[i].fid.fsid.val[0] &&
> - !objects[i].fid.fsid.val[1]) {
> - tst_res(TCONF,
> - "FAN_REPORT_FID not supported on "
> - "filesystem type %s",
> - tst_device->fs_type);
> - return 1;
> - }
This should not have been here.
Should here been removed by the patch that added
fanotify_report_fid_supported_on_fs().
But I understand the confusion.
FAN_REPORT_FID support on fs can only be tested by fanotify_init followed
by fanotify_mark, even though FAN_REPORT_FID is a fanotify_init flag.
Nevertheless, fanotify_report_fid_supported_on_fs() has already verified
that we can add a mark on filesystem with FAN_REPORT_FID group, so
this check here is not needed anymore.
Thanks,
Amir.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
` (6 preceding siblings ...)
2020-11-26 21:47 ` [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
@ 2020-11-27 15:36 ` Amir Goldstein
7 siblings, 0 replies; 14+ messages in thread
From: Amir Goldstein @ 2020-11-27 15:36 UTC (permalink / raw)
To: ltp
On Thu, Nov 26, 2020 at 11:41 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> another cleanup version, hopefully the last.
> Sorry for lengthy comments.
Don't be :)
>
> Changes v3->v4:
> * fixed unwanted tst_brk() (quitting the test too early). In the end it
> was problem just in fanotify01.c and fanotify03.c.
>
> In fanotify13.c it was already
>
> tst_test.c:1316: TINFO: Testing on exfat
> fanotify.h:209: TCONF: filesystem exfat does not support file handles
> ...
> tst_test.c:859: TINFO: Trying FUSE...
> fanotify13.c:161: TINFO: Test #0: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
> fanotify13.c:161: TINFO: Test #1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
> fanotify13.c:161: TINFO: Test #2: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
> fanotify13.c:161: TINFO: Test #3: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
> fanotify13.c:161: TINFO: Test #4: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
> fanotify13.c:161: TINFO: Test #5: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> fanotify13.c:130: TCONF: FAN_REPORT_FID not supported on filesystem type ntfs
>
> * new commit fanotify: Check for FAN_REPORT_FID support on fs
> This one I hesitated, whether keep EOPNOTSUPP also as fallback in fanotify.h
> On all but one test (fanotify01.c) FAN_REPORT_FID was used in all tests.
>
> The only check I kept untouched is this one from fanotify16.c:
> fd_notify = fanotify_init(group->flag, 0);
> if (fd_notify == -1) {
> if (errno == EINVAL) {
> tst_res(TCONF,
> "%s not supported by kernel", group->name);
> return;
> }
>
> tst_brk(TBROK | TERRNO,
> "fanotify_init(%s, 0) failed", group->name);
> }
>
> Because this:
> fanotify16.c:160: TINFO: Test #0: FAN_REPORT_DFID_NAME monitor filesystem for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME not supported by kernel
> fanotify16.c:160: TINFO: Test #1: FAN_REPORT_DFID_NAME monitor directories for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME not supported by kernel
> fanotify16.c:160: TINFO: Test #2: FAN_REPORT_DIR_FID monitor filesystem for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DIR_FID not supported by kernel
> fanotify16.c:160: TINFO: Test #3: FAN_REPORT_DIR_FID monitor directories for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DIR_FID not supported by kernel
> fanotify16.c:160: TINFO: Test #4: FAN_REPORT_DFID_FID monitor filesystem for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_FID not supported by kernel
> fanotify16.c:160: TINFO: Test #5: FAN_REPORT_DFID_FID monitor directories for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_FID not supported by kernel
> fanotify16.c:160: TINFO: Test #6: FAN_REPORT_DFID_NAME_FID monitor filesystem for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME_FID not supported by kernel
> fanotify16.c:160: TINFO: Test #7: FAN_REPORT_DFID_NAME_FID monitor directories for create/delete/move/open/close
> fanotify16.c:166: TCONF: FAN_REPORT_DFID_NAME_FID not supported by kernel
>
> would be replaced by single message which is misleading:
> fanotify16.c:163: TCONF: FAN_REPORT_NAME not supported in kernel?
> if I use
> fd_notify = SAFE_FANOTIFY_INIT(group->flag, 0);
> + that problem with skipping what we don't want to skip (although here
> are skipped all and on old kernel I get
> fanotify.h:342: TCONF: FAN_REPORT_FID not supported in kernel?
That's because you should have tested FAN_REPORT_NAME before
testing FAN_REPORT_FID.
Also, instead of testing FAN_REPORT_NAME you can test for
(flags & FAN_REPORT_DIR_FID) both flags were added together
in the same kernel but FAN_REPORT_NAME cannot be used without
FAN_REPORT_DIR_FID.
>
> and on new kernel problematic fs are skipped anyway:
> fanotify.h:363: TCONF: FAN_REPORT_FID not supported on exfat filesystem
>
> Feel free to suggest what to do or simply send a following patch.
>
I am not sure if my answers addressed all the problems you encountered
with fanotify16. Feel free to leave the remaining problem out of the cleanup
and I will fix it later.
> In following commit "fanotify: Introduce SAFE_FANOTIFY_MARK() macro".
> I'd prefer to keep it (fallback if we forget to add a check), but probably instead of
> "some FAN_REPORT_* flag not supported on %s filesystem",
> there should be
> "FAN_REPORT_FID flag not supported on %s filesystem",
> But on my machine with 5.10.0-rc4-1.gea0f69f-default, this failed on FAN_REPORT_DFID_NAME
> (synonym for (FAN_REPORT_DIR_FID|FAN_REPORT_NAME) => no FAN_REPORT_FID,
> so this would be misleading:
> fanotify16.c:160: TINFO: Test #0: FAN_REPORT_DFID_NAME monitor filesystem for create/delete/move/open/close
> fanotify16.c:177: TCONF: FAN_REPORT_FID not supported on exfat filesystem
>
> * more cleanup in commit "fanotify: Check FAN_REPORT_{FID,NAME} support"
>
> Other changes (suggested mostly by Cyril):
> * rename s/support_exec_events/exec_events_unsupported/
>
> * Add "require_" prefix for functions which use tst_brk()
>
> * use tst_res_() and tst_brk_() for safe_*() functions
>
> BTW fanotify16.c use .dev_fs_flags = TST_FS_SKIP_FUSE, this could be
> added also into fanotify13.c (it'd avoid running ntfs).
>
> If LTP had something like TST_FS_SKIP_MICROSOFT (TST_FS_SKIP_FUSE +
> exfat), could get rid of "FAN_REPORT_FID not supported on foo
> filesystem" testing. But it's already implemented, so it's just a note
> to be ignored.
>
It is better to check support by trying unless there is a very good reason
for special casing filesystems. I don't remember what was the special case
for ntfs that test support checks did not catch.
I guess the TST_FS_SKIP_FUSE flag may not be needed after your fixes?
Thanks,
Amir.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-11-27 15:36 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 21:41 [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 1/6] fanotify12: Drop incorrect hint Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 2/6] fanotify: Handle supported features checks in setup() Petr Vorel
2020-11-27 13:20 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 3/6] fanotify: Check for FAN_REPORT_FID support on fs Petr Vorel
2020-11-27 13:47 ` Amir Goldstein
2020-11-27 14:26 ` Petr Vorel
2020-11-26 21:41 ` [LTP] [PATCH v4 4/6] fanotify: Introduce SAFE_FANOTIFY_MARK() macro Petr Vorel
2020-11-27 15:17 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 5/6] fanotify: Check FAN_REPORT_{FID, NAME} support Petr Vorel
2020-11-27 14:22 ` Amir Goldstein
2020-11-26 21:41 ` [LTP] [PATCH v4 6/6] fanotify: Add a pedantic check for return value Petr Vorel
2020-11-26 21:47 ` [LTP] [PATCH v4 0/6] Introduce SAFE_FANOTIFY_MARK() macro + cleanup Petr Vorel
2020-11-27 15:36 ` Amir Goldstein
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.