diff mbox series

[2/2] landlock01: used fixed size for rule_small_size

Message ID 20241105041326.18531-2-liwang@redhat.com
State Superseded
Headers show
Series [1/2] lapi: Add new field socket scoping to landlock_ruleset_attr | expand

Commit Message

Li Wang Nov. 5, 2024, 4:13 a.m. UTC
The landlock01 test is failing on kernel v6.12-rc6 with the
following error:

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

Previously, rule_small_size was calculated conditionally based
on the presence of the handled_access_net field in the struct
landlock_ruleset_attr.

However, the kernel's landlock_create_ruleset() function still uses
the size up to handled_access_fs to determine the minimal acceptable
size for backward compatibility, regardless of any new fields added.

According to the Landlock maintainer[1], this behavior will remain
for backward compatibility reasons. Therefore, it's unnecessary to
conditionally adjust rule_small_size based on new fields.

This patch simplifies the test by setting rule_small_size to
'sizeof(__u64) - 1', which effectively tests the minimal size based
on handled_access_fs.

[1] https://lists.linux.it/pipermail/ltp/2024-July/039084.html

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/landlock/landlock01.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Wei Gao Nov. 5, 2024, 10:43 a.m. UTC | #1
On Tue, Nov 05, 2024 at 12:13:25PM +0800, Li Wang wrote:
> The landlock01 test is failing on kernel v6.12-rc6 with the
> following error:
> 
>   landlock01.c:49: TFAIL: Size is too small expected EINVAL: ENOMSG (42)
> 
> Previously, rule_small_size was calculated conditionally based
> on the presence of the handled_access_net field in the struct
> landlock_ruleset_attr.
> 
> However, the kernel's landlock_create_ruleset() function still uses
> the size up to handled_access_fs to determine the minimal acceptable
> size for backward compatibility, regardless of any new fields added.
> 
> According to the Landlock maintainer[1], this behavior will remain
> for backward compatibility reasons. Therefore, it's unnecessary to
> conditionally adjust rule_small_size based on new fields.
> 
> This patch simplifies the test by setting rule_small_size to
> 'sizeof(__u64) - 1', which effectively tests the minimal size based
> on handled_access_fs.
> 
> [1] https://lists.linux.it/pipermail/ltp/2024-July/039084.html
> 
> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  testcases/kernel/syscalls/landlock/landlock01.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/landlock/landlock01.c b/testcases/kernel/syscalls/landlock/landlock01.c
> index 083685c64..c375e5154 100644
> --- a/testcases/kernel/syscalls/landlock/landlock01.c
> +++ b/testcases/kernel/syscalls/landlock/landlock01.c
> @@ -62,11 +62,17 @@ static void setup(void)
>  
>  	rule_size = sizeof(struct landlock_ruleset_attr);
>  
> -#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
Since we do not need HAVE_STRUCT_LANDLOCK_RULESET_ATTR_HANDLED_ACCESS_NET anymore, so i suppose we can remove
following change in configure.ac?
 AC_CHECK_MEMBERS([struct landlock_ruleset_attr.handled_access_net],,,[#include <linux/landlock.h>])
> +	/*
> +	 * Kernel introduces two new fields 'handled_access_net' and 'scoped'
> +	 * in the structure 'landlock_ruleset_attr'. However, in the function
> +	 * 'landlock_create_ruleset()', it still uses the first field
> +	 * 'handled_access_fs' to calculate the minimal size for backward
> +	 * compatibility reason.
> +	 *
> +	 * Therefore, here test 'sizeof(__u64) - 1' is sufficient to determine
> +	 * the minimum size for 'rule_small_size'.
> +	 */
> +	rule_small_size = sizeof(__u64) - 1;
>  
>  	rule_big_size = SAFE_SYSCONF(_SC_PAGESIZE) + 1;
>  }
> -- 
> 2.47.0
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp
Li Wang Nov. 5, 2024, noon UTC | #2
On Tue, Nov 5, 2024 at 6:43 PM Wei Gao <wegao@suse.com> wrote:

> On Tue, Nov 05, 2024 at 12:13:25PM +0800, Li Wang wrote:
> > The landlock01 test is failing on kernel v6.12-rc6 with the
> > following error:
> >
> >   landlock01.c:49: TFAIL: Size is too small expected EINVAL: ENOMSG (42)
> >
> > Previously, rule_small_size was calculated conditionally based
> > on the presence of the handled_access_net field in the struct
> > landlock_ruleset_attr.
> >
> > However, the kernel's landlock_create_ruleset() function still uses
> > the size up to handled_access_fs to determine the minimal acceptable
> > size for backward compatibility, regardless of any new fields added.
> >
> > According to the Landlock maintainer[1], this behavior will remain
> > for backward compatibility reasons. Therefore, it's unnecessary to
> > conditionally adjust rule_small_size based on new fields.
> >
> > This patch simplifies the test by setting rule_small_size to
> > 'sizeof(__u64) - 1', which effectively tests the minimal size based
> > on handled_access_fs.
> >
> > [1] https://lists.linux.it/pipermail/ltp/2024-July/039084.html
> >
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > ---
> >  testcases/kernel/syscalls/landlock/landlock01.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/landlock/landlock01.c
> b/testcases/kernel/syscalls/landlock/landlock01.c
> > index 083685c64..c375e5154 100644
> > --- a/testcases/kernel/syscalls/landlock/landlock01.c
> > +++ b/testcases/kernel/syscalls/landlock/landlock01.c
> > @@ -62,11 +62,17 @@ static void setup(void)
> >
> >       rule_size = sizeof(struct landlock_ruleset_attr);
> >
> > -#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
> Since we do not need HAVE_STRUCT_LANDLOCK_RULESET_ATTR_HANDLED_ACCESS_NET
> anymore, so i suppose we can remove
> following change in configure.ac?
>

Theirecally that's correct, but I see Andrea achieving V2 patchset for
landlock.
Let's not delete the macro yet but check which method works first.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/landlock/landlock01.c b/testcases/kernel/syscalls/landlock/landlock01.c
index 083685c64..c375e5154 100644
--- a/testcases/kernel/syscalls/landlock/landlock01.c
+++ b/testcases/kernel/syscalls/landlock/landlock01.c
@@ -62,11 +62,17 @@  static void setup(void)
 
 	rule_size = sizeof(struct landlock_ruleset_attr);
 
-#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
+	/*
+	 * Kernel introduces two new fields 'handled_access_net' and 'scoped'
+	 * in the structure 'landlock_ruleset_attr'. However, in the function
+	 * 'landlock_create_ruleset()', it still uses the first field
+	 * 'handled_access_fs' to calculate the minimal size for backward
+	 * compatibility reason.
+	 *
+	 * Therefore, here test 'sizeof(__u64) - 1' is sufficient to determine
+	 * the minimum size for 'rule_small_size'.
+	 */
+	rule_small_size = sizeof(__u64) - 1;
 
 	rule_big_size = SAFE_SYSCONF(_SC_PAGESIZE) + 1;
 }