linux-kernel - Re: [PATCH] selftests: lsm: Refactor `flags_overset_lsm_set_self

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52cc8e51-9e85-465b-8ee3-63a7a0a42951@linuxfoundation.org>
Date: Thu, 14 Nov 2024 09:25:09 -0700
From: Shuah Khan <skhan@...uxfoundation.org>
To: Amit Vadhavana <av2082000@...il.com>, paul@...l-moore.com,
 jmorris@...ei.org, serge@...lyn.com, casey@...aufler-ca.com, shuah@...nel.org
Cc: ricardo@...liere.net, linux-kernel-mentees@...ts.linux.dev,
 linux-security-module@...r.kernel.org, linux-kselftest@...r.kernel.org,
 linux-kernel@...r.kernel.org, Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH] selftests: lsm: Refactor
 `flags_overset_lsm_set_self_attr` test

On 11/12/24 11:28, Amit Vadhavana wrote:
> - Remove unnecessary `tctx` variable, use `ctx` directly.
> - Simplified code with no functional changes.
> 

I would rephrase the short to simply say Remove unused variable,
as refactor implies more extensive changes than what this patch
is actually doing.

Please write complete sentences instead of bullet points in the
change log.

How did you find this problem? Do include the details on how
in the change log.

> Signed-off-by: Amit Vadhavana <av2082000@...il.com>
> ---
>   tools/testing/selftests/lsm/lsm_set_self_attr_test.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> index 66dec47e3ca3..732e89fe99c0 100644
> --- a/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> +++ b/tools/testing/selftests/lsm/lsm_set_self_attr_test.c
> @@ -56,16 +56,15 @@ TEST(flags_zero_lsm_set_self_attr)
>   TEST(flags_overset_lsm_set_self_attr)
>   {
>   	const long page_size = sysconf(_SC_PAGESIZE);
> -	char *ctx = calloc(page_size, 1);
> +	struct lsm_ctx *ctx = calloc(page_size, 1);

Why not name this tctx and avoid changes to the ASSERT_EQs
below?

>   	__u32 size = page_size;
> -	struct lsm_ctx *tctx = (struct lsm_ctx *)ctx;
>   
>   	ASSERT_NE(NULL, ctx);
>   	if (attr_lsm_count()) {
> -		ASSERT_LE(1, lsm_get_self_attr(LSM_ATTR_CURRENT, tctx, &size,
> +		ASSERT_LE(1, lsm_get_self_attr(LSM_ATTR_CURRENT, ctx, &size,
>   					       0));
>   	}
> -	ASSERT_EQ(-1, lsm_set_self_attr(LSM_ATTR_CURRENT | LSM_ATTR_PREV, tctx,
> +	ASSERT_EQ(-1, lsm_set_self_attr(LSM_ATTR_CURRENT | LSM_ATTR_PREV, ctx,
>   					size, 0));
>   
>   	free(ctx);

You have to change this tctx for sure.

With these changes:

Reviewed-by: Shuah Khan <skhan@...uxfoundation.org>

Paul, James,

Please do let me know if you would me to take this through
kselftest tree.

thanks,
-- Shuah