Re: [LTP] [PATCH v3 05/11] Add landlock01 test

[Petr Vorel <pvorel@suse.cz>]

> Hi Petr,

> On Fri, Jul 12, 2024 at 3:58 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Andrea, Li,

> > ...
> > > >> First, this fails at least on various kernel versions (tested:
> > Tumbleweed
> > > >> 6.10.0-rc7-3.g92abc10-default, and SLE15-SP4 5.14.21 with
> > lsm=landlock and
> > > >> Debian 6.6.15-amd64):

> > > >> landlock01.c:49: TFAIL: Size is too small expected EINVAL: ENOMSG (42)

> > > >> Is it a kernel bug or a test bug?


> > > > You probably need to check the `/usr/include/linux/landlock.h` header
> > file
> > > > exist,
> > > > and to see if 'struct landlock_ruleset_attr' contains the new field
> > > > 'handled_access_net'.

> > > > If not exist or does not contain that, the test defines 'struct
> > > > landlock_ruleset_attr'
> > > > in lapi/landlock.h which contains handled_access_net directly, this is
> > > > likely the
> > > > root cause lead test failed on your box.

> > > And, if the header file does not exist, the macro condition will choose
> > to
> > > use 'rule_size - 1',
> > > and that caused the ENOMSG error during test on the newer kernel.

> > > #ifdef HAVE_STRUCT_LANDLOCK_RULESET_ATTR_HANDLED_ACCESS_NET
> > >     rule_small_size = rule_size - sizeof(uint64_t) - 1;
> > > #else
> > >     rule_small_size = rule_size - 1;
> > > #endif

> > > So to keep the kernel-headers and running kernel version consistent
> > should
> > > be required
> > > for the landlock01 test. Otherwise the #ifdef possibly won't work
> > correctly.

> > FYI Having inconsistent kernel headers and running kernel would be a
> > problem for
> > more LTP tests than just landlock01 (basically many tests which have
> > autotools
> > check). But this can be problematic for some development (e.g. linux-next).
> > Therefore we at least assume UAPI headers shouldn't be newer than running
> > kernel, see https://lore.kernel.org/ltp/ZJP_qPeJ37H4qhEN@yuki/.


> Yes, I agree on this.

> As landlock01 uses the macro I pointed out in the last email,
> it is almost unable to set a correct rule_small_size w/o
> variants of kernel-headers.

> So I still think just simply set the 'rule_small_size' to 'sizeof(__u64) -
> 1;'
> will make life easier but Andrea has a different perspective on that.

It looks to me also better, but let's ask others :).
@Jan, @Cyril WDYT?

> Anyway, I would leave this to Andrea (an excellent black-box tester)
> for more struggling. lol~

Lol :).
I suppose userspace developers which use raw syscalls are often forced to
look into kernel sources (man pages are sparse).

Kind regards,
Petr

> > > I guess we might have to resolve this on the test side.

> > Trying to compare versions <linux/version.h> could be used:

> > #define LINUX_VERSION_CODE 395008
> > #define KERNEL_VERSION(a,b,c) (((a) << 16) + ((b) << 8) + ((c) > 255 ? 255
> > : (c)))
> > #define LINUX_VERSION_MAJOR 6
> > #define LINUX_VERSION_PATCHLEVEL 7
> > #define LINUX_VERSION_SUBLEVEL 0

> > We already use KERNEL_VERSION() in kdump and device-drivers tests.


> It could work by adding more kernel judgment for the macro definition,
> but a little bit of a mess IMHO.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp