On Fri, Apr 30, 2021 at 11:24:00AM -0400, Richard Guy Briggs wrote:
> On 2021-04-23 13:10, Christian Brauner wrote:
> > From: Christian Brauner <christian.brauner@xxxxxxxxxx>
> >
> > Test that openat2() rejects unknown flags in the upper 32 bit range.
> >
> > Cc: Richard Guy Briggs <rgb@xxxxxxxxxx>
> > Cc: Aleksa Sarai <cyphar@xxxxxxxxxx>
> > Cc: linux-fsdevel@xxxxxxxxxxxxxxx
> > Signed-off-by: Christian Brauner <christian.brauner@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/openat2/openat2_test.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c
> > index 381d874cce99..7379e082a994 100644
> > --- a/tools/testing/selftests/openat2/openat2_test.c
> > +++ b/tools/testing/selftests/openat2/openat2_test.c
> > @@ -155,7 +155,7 @@ struct flag_test {
> > int err;
> > };
> >
> > -#define NUM_OPENAT2_FLAG_TESTS 24
> > +#define NUM_OPENAT2_FLAG_TESTS 25
> >
> > void test_openat2_flags(void)
> > {
> > @@ -229,6 +229,11 @@ void test_openat2_flags(void)
> > { .name = "invalid how.resolve and O_PATH",
> > .how.flags = O_PATH,
> > .how.resolve = 0x1337, .err = -EINVAL },
> > +
> > + /* Invalid flags in the upper 32 bits must be rejected. */
> > + { .name = "invalid flags (1 << 63)",
> > + .how.flags = O_RDONLY | (1ULL << 63),
> > + .how.resolve = 0, .err = -EINVAL },
>
> This doesn't appear to specifically test for flags over 32 bits. It
> appears to test for flags not included in VALID_OPEN_FLAGS.
>
> "1ULL << 2" would accomplish the same thing, as would "1ULL << 31" due
> to the unused flags in the bottom 32 bits.
>
> The test appears to be useful, but misnamed.
I mean we can name it test "currently unknown upper bit".
>
> If a new flag was added at 1ULL << 33, this test wouldn't notice and it
It isn't supposed to notice because it's a known flag. If we add
#define O_FANCY (1ULL << 63)
this test should fail and either would need to be adapted or more likely
be dropped since all bits are taken apparently.
> would still get dropped in build_open_flags() when flags gets assigned
> to op->open_flags.
I didn't intend to add a test whether flags are silently dropped. I
intended to add a test whether any currently unkown bit in the upper 32
bits is loudly rejected instead of silently ignored.
I may misunderstand what kind of test you would like to see here.
Christian
