[LTP] [PATCH v2] lsm: fix overset attr test

[LTP] [PATCH v2] lsm: fix overset attr test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

LSM(s) usually handle their own internal errors in a different way,
so the right way to check if they return error, is to verify that the
common return value is -1. This is the max we can do, since errno might
vary according to the LSM implementation.

At the same time, overset attr test is _not_ checking if attr is
overset, but rather checking if attr is out-of-bounds, considering OR
operator as a valid way to generate an invalid value with
LSM_ATTR_CURRENT. This is not correct, since any OR operation using
LSM_ATTR_CURRENT will generate a valid value for the LSM(s) code. So we
remove this test that doesn't make much sense at the moment and replace
it with an "invalid attr test" instead.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
This patch will fix all false positive errors, where LSM(s) might
be implemented in a different way. We just skip errno check.

This will also fix:
https://openqa.opensuse.org/tests/5087893#step/lsm_set_self_attr01/8
---
Changes in v2:
- remove exp_errno from struct
- change attr overset test
- Link to v1: https://lore.kernel.org/r/20250604-lsm_fix_attr_is_overset-v1-1-46ff86423a14@suse.com
---
 testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
index caccdda7ecf2edaac1fa8e2dc2ccdd0aff020804..cde9c2e706ed607024dff362b7ff00cbcef1d6a5 100644
--- a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
+++ b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
@@ -23,28 +23,24 @@ static struct tcase {
 	struct lsm_ctx **ctx;
 	uint32_t *size;
 	uint32_t flags;
-	int exp_errno;
 	char *msg;
 } tcases[] = {
 	{
 		.attr = LSM_ATTR_CURRENT,
 		.ctx = &ctx_null,
 		.size = &ctx_size,
-		.exp_errno = EFAULT,
 		.msg = "ctx is NULL",
 	},
 	{
 		.attr = LSM_ATTR_CURRENT,
 		.ctx = &ctx,
 		.size = &ctx_size_small,
-		.exp_errno = EINVAL,
 		.msg = "size is too small",
 	},
 	{
 		.attr = LSM_ATTR_CURRENT,
 		.ctx = &ctx,
 		.size = &ctx_size_big,
-		.exp_errno = E2BIG,
 		.msg = "size is too big",
 	},
 	{
@@ -52,15 +48,13 @@ static struct tcase {
 		.ctx = &ctx,
 		.size = &ctx_size,
 		.flags = 1,
-		.exp_errno = EINVAL,
 		.msg = "flags must be zero",
 	},
 	{
-		.attr = LSM_ATTR_CURRENT | LSM_ATTR_EXEC,
+		.attr = -1000,
 		.ctx = &ctx,
 		.size = &ctx_size,
-		.exp_errno = EINVAL,
-		.msg = "attr is overset",
+		.msg = "attr is invalid",
 	}
 };
 
@@ -77,9 +71,9 @@ static void run(unsigned int n)
 	ctx_size_small = 1;
 	ctx_size_big = ctx_size + 1;
 
-	TST_EXP_FAIL(lsm_set_self_attr(tc->attr, *tc->ctx, *tc->size, tc->flags),
-	      tc->exp_errno,
-	      "%s", tc->msg);
+	TST_EXP_EXPR(lsm_set_self_attr(
+		tc->attr, *tc->ctx, *tc->size, tc->flags) == -1,
+		"%s", tc->msg);
 }
 
 static void setup(void)

---
base-commit: 3ef4bac1dd253628dfdb8a823b51ae0d24fa0616
change-id: 20250604-lsm_fix_attr_is_overset-132c44f10767

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


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

Re: [LTP] [PATCH v2] lsm: fix overset attr test

[Petr Vorel]

Hi Andrea,

> LSM(s) usually handle their own internal errors in a different way,
> so the right way to check if they return error, is to verify that the
> common return value is -1. This is the max we can do, since errno might
> vary according to the LSM implementation.

> At the same time, overset attr test is _not_ checking if attr is
> overset, but rather checking if attr is out-of-bounds, considering OR
> operator as a valid way to generate an invalid value with
> LSM_ATTR_CURRENT. This is not correct, since any OR operation using
> LSM_ATTR_CURRENT will generate a valid value for the LSM(s) code. So we
> remove this test that doesn't make much sense at the moment and replace
> it with an "invalid attr test" instead.

Thanks for the fix, LGTM.

Fixes: ad4ab6ce4f ("Add lsm_set_self_attr01 test")
Acked-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> This patch will fix all false positive errors, where LSM(s) might
> be implemented in a different way. We just skip errno check.

> This will also fix:
> https://openqa.opensuse.org/tests/5087893#step/lsm_set_self_attr01/8
> ---
> Changes in v2:
> - remove exp_errno from struct
> - change attr overset test
> - Link to v1: https://lore.kernel.org/r/20250604-lsm_fix_attr_is_overset-v1-1-46ff86423a14@suse.com
> ---
>  testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)

> diff --git a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> index caccdda7ecf2edaac1fa8e2dc2ccdd0aff020804..cde9c2e706ed607024dff362b7ff00cbcef1d6a5 100644
> --- a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> +++ b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> @@ -23,28 +23,24 @@ static struct tcase {
>  	struct lsm_ctx **ctx;
>  	uint32_t *size;
>  	uint32_t flags;
> -	int exp_errno;
>  	char *msg;
>  } tcases[] = {
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx_null,
>  		.size = &ctx_size,
> -		.exp_errno = EFAULT,
>  		.msg = "ctx is NULL",
>  	},
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx,
>  		.size = &ctx_size_small,
> -		.exp_errno = EINVAL,
>  		.msg = "size is too small",
>  	},
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx,
>  		.size = &ctx_size_big,
> -		.exp_errno = E2BIG,
>  		.msg = "size is too big",
>  	},
>  	{
> @@ -52,15 +48,13 @@ static struct tcase {
>  		.ctx = &ctx,
>  		.size = &ctx_size,
>  		.flags = 1,
> -		.exp_errno = EINVAL,
>  		.msg = "flags must be zero",
>  	},
>  	{
> -		.attr = LSM_ATTR_CURRENT | LSM_ATTR_EXEC,
> +		.attr = -1000,
>  		.ctx = &ctx,
>  		.size = &ctx_size,
> -		.exp_errno = EINVAL,
> -		.msg = "attr is overset",
> +		.msg = "attr is invalid",
>  	}
>  };

> @@ -77,9 +71,9 @@ static void run(unsigned int n)
>  	ctx_size_small = 1;
>  	ctx_size_big = ctx_size + 1;

> -	TST_EXP_FAIL(lsm_set_self_attr(tc->attr, *tc->ctx, *tc->size, tc->flags),
> -	      tc->exp_errno,
> -	      "%s", tc->msg);
> +	TST_EXP_EXPR(lsm_set_self_attr(
> +		tc->attr, *tc->ctx, *tc->size, tc->flags) == -1,
> +		"%s", tc->msg);
>  }

Re: [LTP] [PATCH v2] lsm: fix overset attr test

[Petr Vorel]

Hi Andrea,

> LSM(s) usually handle their own internal errors in a different way,
> so the right way to check if they return error, is to verify that the
> common return value is -1. This is the max we can do, since errno might
> vary according to the LSM implementation.

> At the same time, overset attr test is _not_ checking if attr is
> overset, but rather checking if attr is out-of-bounds, considering OR
> operator as a valid way to generate an invalid value with
> LSM_ATTR_CURRENT. This is not correct, since any OR operation using
> LSM_ATTR_CURRENT will generate a valid value for the LSM(s) code. So we
> remove this test that doesn't make much sense at the moment and replace
> it with an "invalid attr test" instead.

Thanks for the fix, LGTM.

Fixes: ad4ab6ce4f ("Add lsm_set_self_attr01 test")
Acked-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
> This patch will fix all false positive errors, where LSM(s) might
> be implemented in a different way. We just skip errno check.

> This will also fix:
> https://openqa.opensuse.org/tests/5087893#step/lsm_set_self_attr01/8
> ---
> Changes in v2:
> - remove exp_errno from struct
> - change attr overset test
> - Link to v1: https://lore.kernel.org/r/20250604-lsm_fix_attr_is_overset-v1-1-46ff86423a14@suse.com
> ---
>  testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)

> diff --git a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> index caccdda7ecf2edaac1fa8e2dc2ccdd0aff020804..cde9c2e706ed607024dff362b7ff00cbcef1d6a5 100644
> --- a/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> +++ b/testcases/kernel/syscalls/lsm/lsm_set_self_attr01.c
> @@ -23,28 +23,24 @@ static struct tcase {
>  	struct lsm_ctx **ctx;
>  	uint32_t *size;
>  	uint32_t flags;
> -	int exp_errno;
>  	char *msg;
>  } tcases[] = {
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx_null,
>  		.size = &ctx_size,
> -		.exp_errno = EFAULT,
>  		.msg = "ctx is NULL",
>  	},
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx,
>  		.size = &ctx_size_small,
> -		.exp_errno = EINVAL,
>  		.msg = "size is too small",
>  	},
>  	{
>  		.attr = LSM_ATTR_CURRENT,
>  		.ctx = &ctx,
>  		.size = &ctx_size_big,
> -		.exp_errno = E2BIG,
>  		.msg = "size is too big",
>  	},
>  	{
> @@ -52,15 +48,13 @@ static struct tcase {
>  		.ctx = &ctx,
>  		.size = &ctx_size,
>  		.flags = 1,
> -		.exp_errno = EINVAL,
>  		.msg = "flags must be zero",
>  	},
>  	{
> -		.attr = LSM_ATTR_CURRENT | LSM_ATTR_EXEC,
> +		.attr = -1000,
>  		.ctx = &ctx,
>  		.size = &ctx_size,
> -		.exp_errno = EINVAL,
> -		.msg = "attr is overset",
> +		.msg = "attr is invalid",
>  	}
>  };

> @@ -77,9 +71,9 @@ static void run(unsigned int n)
>  	ctx_size_small = 1;
>  	ctx_size_big = ctx_size + 1;

> -	TST_EXP_FAIL(lsm_set_self_attr(tc->attr, *tc->ctx, *tc->size, tc->flags),
> -	      tc->exp_errno,
> -	      "%s", tc->msg);
> +	TST_EXP_EXPR(lsm_set_self_attr(
> +		tc->attr, *tc->ctx, *tc->size, tc->flags) == -1,
> +		"%s", tc->msg);
>  }

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

Re: [LTP] [PATCH v2] lsm: fix overset attr test

[Cyril Hrubis]

Hi!
> LSM(s) usually handle their own internal errors in a different way,
> so the right way to check if they return error, is to verify that the
> common return value is -1. This is the max we can do, since errno might
> vary according to the LSM implementation.

Is there a small set of errnos that would make sense for each failure?

We have TST_EXP_FAIL_ARR() for filesystem tests for the case where there
are a few different possible error outcomes, maybe the same approach
would work here as well.

> At the same time, overset attr test is _not_ checking if attr is
> overset, but rather checking if attr is out-of-bounds, considering OR
> operator as a valid way to generate an invalid value with
> LSM_ATTR_CURRENT. This is not correct, since any OR operation using
> LSM_ATTR_CURRENT will generate a valid value for the LSM(s) code. So we
> remove this test that doesn't make much sense at the moment and replace
> it with an "invalid attr test" instead.

That part looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

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