From: Petr Vorel <pvorel@suse.cz>
To: Andrea Cervesato <andrea.cervesato@suse.de>
Cc: ltp@lists.linux.it, linux-security-module@vger.kernel.org,
Paul Moore <paul@paul-moore.com>,
Kees Cook <keescook@chromium.org>
Subject: Re: [LTP] [PATCH v2] lsm: fix overset attr test
Date: Thu, 5 Jun 2025 11:58:26 +0200 [thread overview]
Message-ID: <20250605095826.GB1206250@pevik> (raw)
In-Reply-To: <20250605-lsm_fix_attr_is_overset-v2-1-dd10ddb04238@suse.com>
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);
> }
WARNING: multiple messages have this Message-ID (diff)
From: Petr Vorel <pvorel@suse.cz>
To: Andrea Cervesato <andrea.cervesato@suse.de>
Cc: linux-security-module@vger.kernel.org,
Paul Moore <paul@paul-moore.com>,
ltp@lists.linux.it, Kees Cook <keescook@chromium.org>
Subject: Re: [LTP] [PATCH v2] lsm: fix overset attr test
Date: Thu, 5 Jun 2025 11:58:26 +0200 [thread overview]
Message-ID: <20250605095826.GB1206250@pevik> (raw)
In-Reply-To: <20250605-lsm_fix_attr_is_overset-v2-1-dd10ddb04238@suse.com>
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
next prev parent reply other threads:[~2025-06-05 9:58 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-05 8:59 [LTP] [PATCH v2] lsm: fix overset attr test Andrea Cervesato
2025-06-05 9:58 ` Petr Vorel [this message]
2025-06-05 9:58 ` Petr Vorel
2025-06-06 9:09 ` Cyril Hrubis
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250605095826.GB1206250@pevik \
--to=pvorel@suse.cz \
--cc=andrea.cervesato@suse.de \
--cc=keescook@chromium.org \
--cc=linux-security-module@vger.kernel.org \
--cc=ltp@lists.linux.it \
--cc=paul@paul-moore.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.