| Message ID | 20230710141403.1155151-1-amir73il@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | fanotify14: Test disallow sb/mount mark on anonymous pipe | expand |
Hi Amir, > This case was retroactively disallowed. > This test is meant to encourage the backporting of commit 69562eb0bd3e > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to > all stable kernels. > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > --- > Petr, > This tests for a behavior that we consider broken since the dawn of > fanotify. > The fix was merged to v6.5-rc1. > I've already posted backport patches for kernels > v5.0. > I am not planning to post backport patches for older kernels. I see https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/ I'll suggest to wait till Greg releases the backport (should be quick enough). > Even though the two new test cases do not use FAN_REPORT_FID, > fanotify14 requires FAN_REPORT_FID, so it is not going to run these > test cases on kernel < v5.1 anyway. > Thanks, > Amir. > .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > index bfa0349fe..063a9f96f 100644 > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > @@ -19,6 +19,9 @@ > * > * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir > * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask > + * > + * The pipes test cases are regression tests for commit: > + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs > */ > #define _GNU_SOURCE > @@ -40,6 +43,7 @@ > #define FLAGS_DESC(flags) {(flags), (#flags)} > +static int pipes[2] = {-1, -1}; > static int fanotify_fd; > static int fan_report_target_fid_unsupported; > static int ignore_mark_unsupported; > @@ -60,6 +64,7 @@ static struct test_case_t { > /* when mask.flags == 0, fanotify_init() is expected to fail */ > struct test_case_flags_t mask; > int expected_errno; > + int *pfd; This produces warnings: fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] 70 | {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, | ^ fanotify14.c:67:14: note: ‘pfd’ declared here 67 | int *pfd; | ^~~ fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] 73 | {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, | ^ fanotify14.c:67:14: note: ‘pfd’ declared here 67 | int *pfd; | ^~~ Could you please fix them? I guess pfd must be NULL when unused. The rest LGTM. Kind regards, Petr ...
On Mon, Jul 10, 2023 at 6:50 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Amir, > > > This case was retroactively disallowed. > > > This test is meant to encourage the backporting of commit 69562eb0bd3e > > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to > > all stable kernels. > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > --- > > > Petr, > > > This tests for a behavior that we consider broken since the dawn of > > fanotify. > > > The fix was merged to v6.5-rc1. > > I've already posted backport patches for kernels > v5.0. > > I am not planning to post backport patches for older kernels. > > I see > https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/ > > I'll suggest to wait till Greg releases the backport (should be quick enough). > ok. > > Even though the two new test cases do not use FAN_REPORT_FID, > > fanotify14 requires FAN_REPORT_FID, so it is not going to run these > > test cases on kernel < v5.1 anyway. > > > Thanks, > > Amir. > > > .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > > index bfa0349fe..063a9f96f 100644 > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > > @@ -19,6 +19,9 @@ > > * > > * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir > > * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask > > + * > > + * The pipes test cases are regression tests for commit: > > + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs > > */ > > > #define _GNU_SOURCE > > @@ -40,6 +43,7 @@ > > > #define FLAGS_DESC(flags) {(flags), (#flags)} > > > +static int pipes[2] = {-1, -1}; > > static int fanotify_fd; > > static int fan_report_target_fid_unsupported; > > static int ignore_mark_unsupported; > > @@ -60,6 +64,7 @@ static struct test_case_t { > > /* when mask.flags == 0, fanotify_init() is expected to fail */ > > struct test_case_flags_t mask; > > int expected_errno; > > + int *pfd; > > This produces warnings: > fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > 70 | {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > | ^ > fanotify14.c:67:14: note: ‘pfd’ declared here > 67 | int *pfd; > | ^~~ > fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > 73 | {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > | ^ > fanotify14.c:67:14: note: ‘pfd’ declared here > 67 | int *pfd; > | ^~~ > > Could you please fix them? I guess pfd must be NULL when unused. > ok. but I have to ask, what is the value of explicitly initializing all the old test cases to pfd = NULL? and what is wrong with default NULL initializers? Is it a deliberate decision of LTP to care about this warning? it's a classic pattern for what this patch does - add a new field to test case which all the existing test cases should not care about. Also, I have always seen these warnings for struct tst_test. fanotify14.c:284:1: warning: missing initializer for field 'needs_cmds' of 'struct tst_test' [-Wmissing-field-initializers] 284 | }; | ^ In file included from fanotify14.c:28: ../../../../include/tst_test.h:324:21: note: 'needs_cmds' declared here 324 | const char *const *needs_cmds; | ^~~~~~~~~~ Must we really initialize an empty needs_cmds array for every test? Seems pointless to me, but I may not have the bigger picture. Thanks, Amir.
> On Mon, Jul 10, 2023 at 6:50 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Amir, > > > This case was retroactively disallowed. > > > This test is meant to encourage the backporting of commit 69562eb0bd3e > > > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to > > > all stable kernels. > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > --- > > > Petr, > > > This tests for a behavior that we consider broken since the dawn of > > > fanotify. > > > The fix was merged to v6.5-rc1. > > > I've already posted backport patches for kernels > v5.0. > > > I am not planning to post backport patches for older kernels. > > I see > > https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/ > > I'll suggest to wait till Greg releases the backport (should be quick enough). > ok. > > > Even though the two new test cases do not use FAN_REPORT_FID, > > > fanotify14 requires FAN_REPORT_FID, so it is not going to run these > > > test cases on kernel < v5.1 anyway. > > > Thanks, > > > Amir. > > > .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- > > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > index bfa0349fe..063a9f96f 100644 > > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > @@ -19,6 +19,9 @@ > > > * > > > * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir > > > * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask > > > + * > > > + * The pipes test cases are regression tests for commit: > > > + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs > > > */ > > > #define _GNU_SOURCE > > > @@ -40,6 +43,7 @@ > > > #define FLAGS_DESC(flags) {(flags), (#flags)} > > > +static int pipes[2] = {-1, -1}; > > > static int fanotify_fd; > > > static int fan_report_target_fid_unsupported; > > > static int ignore_mark_unsupported; > > > @@ -60,6 +64,7 @@ static struct test_case_t { > > > /* when mask.flags == 0, fanotify_init() is expected to fail */ > > > struct test_case_flags_t mask; > > > int expected_errno; > > > + int *pfd; > > This produces warnings: > > fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > 70 | {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > | ^ > > fanotify14.c:67:14: note: ‘pfd’ declared here > > 67 | int *pfd; > > | ^~~ > > fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > 73 | {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > | ^ > > fanotify14.c:67:14: note: ‘pfd’ declared here > > 67 | int *pfd; > > | ^~~ > > Could you please fix them? I guess pfd must be NULL when unused. > ok. but I have to ask, > what is the value of explicitly initializing all the old test cases to > pfd = NULL? > and what is wrong with default NULL initializers? > Is it a deliberate decision of LTP to care about this warning? > it's a classic pattern for what this patch does - > add a new field to test case which all the existing test cases > should not care about. Well, we try to avoid warnings in new API tests (and rewriting legacy API tests into new API to cleanup the code). The solution to avoid warnings is to use designated initializers (named initializers), the same way as in ede7f095e ("fanotify10: Use named initializers"): /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ { .init = FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), .expected_errno = EINVAL }, /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ { .init = FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), .expected_errno = EINVAL }, ... { .init = FLAGS_DESC(FAN_CLASS_NOTIF), .mark = FLAGS_DESC(FAN_MARK_FILESYSTEM), .mask = { FAN_ACCESS, "anonymous pipe"}, .expected_errno = EINVAL, .pfd = pipes }, The last one could be without designated initializers, because we pass all struct members, but IMHO it's better for readability. Therefore ideal solution would be to turn the test into designated initializers in separate commit, followed by this change. > Also, I have always seen these warnings for struct tst_test. > fanotify14.c:284:1: warning: missing initializer for field > 'needs_cmds' of 'struct tst_test' [-Wmissing-field-initializers] > 284 | }; > | ^ > In file included from fanotify14.c:28: > ../../../../include/tst_test.h:324:21: note: 'needs_cmds' declared here > 324 | const char *const *needs_cmds; > | ^~~~~~~~~~ These warnings were caused by these GCC bugs (fixed in gcc 12 and backported to gcc 11): https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84685 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82283 Kind regards, Petr > Must we really initialize an empty needs_cmds array for every test? > Seems pointless to me, but I may not have the bigger picture. > Thanks, > Amir.
On Tue, Jul 11, 2023 at 9:34 AM Petr Vorel <pvorel@suse.cz> wrote: > > > On Mon, Jul 10, 2023 at 6:50 PM Petr Vorel <pvorel@suse.cz> wrote: > > > > Hi Amir, > > > > > This case was retroactively disallowed. > > > > > This test is meant to encourage the backporting of commit 69562eb0bd3e > > > > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to > > > > all stable kernels. > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > > --- > > > > > Petr, > > > > > This tests for a behavior that we consider broken since the dawn of > > > > fanotify. > > > > > The fix was merged to v6.5-rc1. > > > > I've already posted backport patches for kernels > v5.0. > > > > I am not planning to post backport patches for older kernels. > > > > I see > > > https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/ > > > > I'll suggest to wait till Greg releases the backport (should be quick enough). > > > > ok. > > > > > Even though the two new test cases do not use FAN_REPORT_FID, > > > > fanotify14 requires FAN_REPORT_FID, so it is not going to run these > > > > test cases on kernel < v5.1 anyway. > > > > > Thanks, > > > > Amir. > > > > > .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- > > > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > index bfa0349fe..063a9f96f 100644 > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > @@ -19,6 +19,9 @@ > > > > * > > > > * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir > > > > * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask > > > > + * > > > > + * The pipes test cases are regression tests for commit: > > > > + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs > > > > */ > > > > > #define _GNU_SOURCE > > > > @@ -40,6 +43,7 @@ > > > > > #define FLAGS_DESC(flags) {(flags), (#flags)} > > > > > +static int pipes[2] = {-1, -1}; > > > > static int fanotify_fd; > > > > static int fan_report_target_fid_unsupported; > > > > static int ignore_mark_unsupported; > > > > @@ -60,6 +64,7 @@ static struct test_case_t { > > > > /* when mask.flags == 0, fanotify_init() is expected to fail */ > > > > struct test_case_flags_t mask; > > > > int expected_errno; > > > > + int *pfd; > > > > This produces warnings: > > > fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > > 70 | {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > > | ^ > > > fanotify14.c:67:14: note: ‘pfd’ declared here > > > 67 | int *pfd; > > > | ^~~ > > > fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > > 73 | {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > > | ^ > > > fanotify14.c:67:14: note: ‘pfd’ declared here > > > 67 | int *pfd; > > > | ^~~ > > > > Could you please fix them? I guess pfd must be NULL when unused. > > > > ok. but I have to ask, > > what is the value of explicitly initializing all the old test cases to > > pfd = NULL? > > and what is wrong with default NULL initializers? > > Is it a deliberate decision of LTP to care about this warning? > > it's a classic pattern for what this patch does - > > add a new field to test case which all the existing test cases > > should not care about. > > Well, we try to avoid warnings in new API tests (and rewriting legacy API tests > into new API to cleanup the code). > > The solution to avoid warnings is to use designated initializers (named > initializers), the same way as in ede7f095e ("fanotify10: Use named > initializers"): > > /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ > { > .init = FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), > .expected_errno = EINVAL > }, > > /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ > { > .init = FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), > .expected_errno = EINVAL > }, > > ... > { > .init = FLAGS_DESC(FAN_CLASS_NOTIF), > .mark = FLAGS_DESC(FAN_MARK_FILESYSTEM), > .mask = { FAN_ACCESS, "anonymous pipe"}, > .expected_errno = EINVAL, > .pfd = pipes > }, > > The last one could be without designated initializers, because we pass all > struct members, but IMHO it's better for readability. > > Therefore ideal solution would be to turn the test into designated initializers > in separate commit, followed by this change. > Ah yes, I remember now. Will do that. > > Also, I have always seen these warnings for struct tst_test. > > > fanotify14.c:284:1: warning: missing initializer for field > > 'needs_cmds' of 'struct tst_test' [-Wmissing-field-initializers] > > 284 | }; > > | ^ > > In file included from fanotify14.c:28: > > ../../../../include/tst_test.h:324:21: note: 'needs_cmds' declared here > > 324 | const char *const *needs_cmds; > > | ^~~~~~~~~~ > > These warnings were caused by these GCC bugs (fixed in gcc 12 and backported to > gcc 11): > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84685 > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82283 > Good to know. Thanks! Amir.
> On Tue, Jul 11, 2023 at 9:34 AM Petr Vorel <pvorel@suse.cz> wrote: > > > On Mon, Jul 10, 2023 at 6:50 PM Petr Vorel <pvorel@suse.cz> wrote: > > > > Hi Amir, > > > > > This case was retroactively disallowed. > > > > > This test is meant to encourage the backporting of commit 69562eb0bd3e > > > > > ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to > > > > > all stable kernels. > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > > > > > --- > > > > > Petr, > > > > > This tests for a behavior that we consider broken since the dawn of > > > > > fanotify. > > > > > The fix was merged to v6.5-rc1. > > > > > I've already posted backport patches for kernels > v5.0. > > > > > I am not planning to post backport patches for older kernels. > > > > I see > > > > https://lore.kernel.org/stable/20230710133205.1154168-1-amir73il@gmail.com/ > > > > I'll suggest to wait till Greg releases the backport (should be quick enough). > > > ok. > > > > > Even though the two new test cases do not use FAN_REPORT_FID, > > > > > fanotify14 requires FAN_REPORT_FID, so it is not going to run these > > > > > test cases on kernel < v5.1 anyway. > > > > > Thanks, > > > > > Amir. > > > > > .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- > > > > > 1 file changed, 30 insertions(+), 2 deletions(-) > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > > index bfa0349fe..063a9f96f 100644 > > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c > > > > > @@ -19,6 +19,9 @@ > > > > > * > > > > > * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir > > > > > * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask > > > > > + * > > > > > + * The pipes test cases are regression tests for commit: > > > > > + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs > > > > > */ > > > > > #define _GNU_SOURCE > > > > > @@ -40,6 +43,7 @@ > > > > > #define FLAGS_DESC(flags) {(flags), (#flags)} > > > > > +static int pipes[2] = {-1, -1}; > > > > > static int fanotify_fd; > > > > > static int fan_report_target_fid_unsupported; > > > > > static int ignore_mark_unsupported; > > > > > @@ -60,6 +64,7 @@ static struct test_case_t { > > > > > /* when mask.flags == 0, fanotify_init() is expected to fail */ > > > > > struct test_case_flags_t mask; > > > > > int expected_errno; > > > > > + int *pfd; > > > > This produces warnings: > > > > fanotify14.c:70:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > > > 70 | {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > > > | ^ > > > > fanotify14.c:67:14: note: ‘pfd’ declared here > > > > 67 | int *pfd; > > > > | ^~~ > > > > fanotify14.c:73:9: warning: missing initializer for field ‘pfd’ of ‘struct test_case_t’ [-Wmissing-field-initializers] > > > > 73 | {FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, > > > > | ^ > > > > fanotify14.c:67:14: note: ‘pfd’ declared here > > > > 67 | int *pfd; > > > > | ^~~ > > > > Could you please fix them? I guess pfd must be NULL when unused. > > > ok. but I have to ask, > > > what is the value of explicitly initializing all the old test cases to > > > pfd = NULL? > > > and what is wrong with default NULL initializers? > > > Is it a deliberate decision of LTP to care about this warning? > > > it's a classic pattern for what this patch does - > > > add a new field to test case which all the existing test cases > > > should not care about. > > Well, we try to avoid warnings in new API tests (and rewriting legacy API tests > > into new API to cleanup the code). > > The solution to avoid warnings is to use designated initializers (named > > initializers), the same way as in ede7f095e ("fanotify10: Use named > > initializers"): > > /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ > > { > > .init = FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), > > .expected_errno = EINVAL > > }, > > /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ > > { > > .init = FLAGS_DESC(FAN_CLASS_PRE_CONTENT | FAN_REPORT_FID), > > .expected_errno = EINVAL > > }, > > ... > > { > > .init = FLAGS_DESC(FAN_CLASS_NOTIF), > > .mark = FLAGS_DESC(FAN_MARK_FILESYSTEM), > > .mask = { FAN_ACCESS, "anonymous pipe"}, > > .expected_errno = EINVAL, > > .pfd = pipes > > }, > > The last one could be without designated initializers, because we pass all > > struct members, but IMHO it's better for readability. > > Therefore ideal solution would be to turn the test into designated initializers > > in separate commit, followed by this change. > Ah yes, I remember now. Will do that. Thanks a lot! Kind regards, Petr > > > Also, I have always seen these warnings for struct tst_test. > > > fanotify14.c:284:1: warning: missing initializer for field > > > 'needs_cmds' of 'struct tst_test' [-Wmissing-field-initializers] > > > 284 | }; > > > | ^ > > > In file included from fanotify14.c:28: > > > ../../../../include/tst_test.h:324:21: note: 'needs_cmds' declared here > > > 324 | const char *const *needs_cmds; > > > | ^~~~~~~~~~ > > These warnings were caused by these GCC bugs (fixed in gcc 12 and backported to > > gcc 11): > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84685 > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82283 > Good to know. > Thanks! > Amir.
diff --git a/testcases/kernel/syscalls/fanotify/fanotify14.c b/testcases/kernel/syscalls/fanotify/fanotify14.c index bfa0349fe..063a9f96f 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify14.c +++ b/testcases/kernel/syscalls/fanotify/fanotify14.c @@ -19,6 +19,9 @@ * * ceaf69f8eadc fanotify: do not allow setting dirent events in mask of non-dir * 8698e3bab4dd fanotify: refine the validation checks on non-dir inode mask + * + * The pipes test cases are regression tests for commit: + * 69562eb0bd3e fanotify: disallow mount/sb marks on kernel internal pseudo fs */ #define _GNU_SOURCE @@ -40,6 +43,7 @@ #define FLAGS_DESC(flags) {(flags), (#flags)} +static int pipes[2] = {-1, -1}; static int fanotify_fd; static int fan_report_target_fid_unsupported; static int ignore_mark_unsupported; @@ -60,6 +64,7 @@ static struct test_case_t { /* when mask.flags == 0, fanotify_init() is expected to fail */ struct test_case_flags_t mask; int expected_errno; + int *pfd; } test_cases[] = { /* FAN_REPORT_FID without class FAN_CLASS_NOTIF is not valid */ {FLAGS_DESC(FAN_CLASS_CONTENT | FAN_REPORT_FID), {}, {}, EINVAL}, @@ -148,6 +153,16 @@ static struct test_case_t { {FLAGS_DESC(FAN_CLASS_NOTIF), FLAGS_DESC(FAN_MARK_FILESYSTEM | FAN_MARK_IGNORE), FLAGS_DESC(FAN_OPEN), EINVAL}, + + /* mount mark on anonymous pipe is not valid */ + {FLAGS_DESC(FAN_CLASS_NOTIF), + FLAGS_DESC(FAN_MARK_MOUNT), + { FAN_ACCESS, "anonymous pipe"}, EINVAL, pipes}, + + /* filesystem mark on anonymous pipe is not valid */ + {FLAGS_DESC(FAN_CLASS_NOTIF), + FLAGS_DESC(FAN_MARK_FILESYSTEM), + { FAN_ACCESS, "anonymous pipe"}, EINVAL, pipes}, }; static void do_test(unsigned int number) @@ -185,11 +200,17 @@ static void do_test(unsigned int number) /* Set mark on non-dir only when expecting error ENOTDIR */ const char *path = tc->expected_errno == ENOTDIR ? FILE1 : MNTPOINT; + int dirfd = AT_FDCWD; + + if (tc->pfd) { + dirfd = tc->pfd[0]; + path = NULL; + } - tst_res(TINFO, "Testing fanotify_mark(FAN_MARK_ADD | %s, %s)", + tst_res(TINFO, "Testing %s with %s", tc->mark.desc, tc->mask.desc); TST_EXP_FD_OR_FAIL(fanotify_mark(fanotify_fd, FAN_MARK_ADD | tc->mark.flags, - tc->mask.flags, AT_FDCWD, path), + tc->mask.flags, dirfd, path), tc->expected_errno); /* @@ -231,12 +252,18 @@ static void do_setup(void) /* Create temporary test file to place marks on */ SAFE_FILE_PRINTF(FILE1, "0"); + /* Create anonymous pipes to place marks on */ + SAFE_PIPE2(pipes, O_CLOEXEC); } static void do_cleanup(void) { if (fanotify_fd > 0) SAFE_CLOSE(fanotify_fd); + if (pipes[0] != -1) + SAFE_CLOSE(pipes[0]); + if (pipes[1] != -1) + SAFE_CLOSE(pipes[1]); } static struct tst_test test = { @@ -251,6 +278,7 @@ static struct tst_test test = { .tags = (const struct tst_tag[]) { {"linux-git", "ceaf69f8eadc"}, {"linux-git", "8698e3bab4dd"}, + {"linux-git", "69562eb0bd3e"}, {} } };
This case was retroactively disallowed. This test is meant to encourage the backporting of commit 69562eb0bd3e ("fanotify: disallow mount/sb marks on kernel internal pseudo fs") to all stable kernels. Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- Petr, This tests for a behavior that we consider broken since the dawn of fanotify. The fix was merged to v6.5-rc1. I've already posted backport patches for kernels > v5.0. I am not planning to post backport patches for older kernels. Even though the two new test cases do not use FAN_REPORT_FID, fanotify14 requires FAN_REPORT_FID, so it is not going to run these test cases on kernel < v5.1 anyway. Thanks, Amir. .../kernel/syscalls/fanotify/fanotify14.c | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)