[LTP] [PATCH 2/7] Add lsm_get_self_attr01 test

[Andrea Cervesato]

Hi Petr,

On 12/18/24 19:55, Petr Vorel wrote:
> Hi Andrea,
>
> [ Cc Casey, the author of the syscalls and kselftest tests ]
>
>> Verify that lsm_get_self_attr syscall is raising errors when invalid
>> data is provided.
>> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> ---
>>   runtest/syscalls                                   |  2 +
>>   testcases/kernel/syscalls/lsm/Makefile             |  7 ++
>>   testcases/kernel/syscalls/lsm/lsm_common.h         | 57 +++++++++++++++
>>   .../kernel/syscalls/lsm/lsm_get_self_attr01.c      | 81 ++++++++++++++++++++++
> You miss the change in .gitignore. You added it in the next commit. Could you
> please before merge rebase, so that it's added for lsm_get_self_attr01 in this
> commit? (in case of some revert).
+1
>>   4 files changed, 147 insertions(+)
>> diff --git a/runtest/syscalls b/runtest/syscalls
>> index 5fd62617df1a116b1d94c57ff30f74693320a2ab..d59faf08a3f36b5f64d56952f69641191c70bf33 100644
>> --- a/runtest/syscalls
>> +++ b/runtest/syscalls
>> @@ -756,6 +756,8 @@ lseek02 lseek02
>>   lseek07 lseek07
>>   lseek11 lseek11
>> +lsm_get_self_attr01 lsm_get_self_attr01
>> +
>>   lstat01 lstat01
>>   lstat01_64 lstat01_64
>>   lstat02 lstat02
>> diff --git a/testcases/kernel/syscalls/lsm/Makefile b/testcases/kernel/syscalls/lsm/Makefile
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..8cf1b9024d8bdebe72408c90fef4b8b84ce9dc4b
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/lsm/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0-or-later
>> +# Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> +
>> +top_srcdir		?= ../../../..
>> +
>> +include $(top_srcdir)/include/mk/testcases.mk
>> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
>> diff --git a/testcases/kernel/syscalls/lsm/lsm_common.h b/testcases/kernel/syscalls/lsm/lsm_common.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..33ddda13720d843907404662e6c6dc72ffac3233
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/lsm/lsm_common.h
>> @@ -0,0 +1,57 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +#ifndef LSM_GET_SELF_ATTR_H
>> +#define LSM_GET_SELF_ATTR_H
>> +
>> +#include "tst_test.h"
>> +#include "lapi/lsm.h"
>> +
>> +static inline struct lsm_ctx *next_ctx(struct lsm_ctx *tctx)
>> +{
>> +	return (struct lsm_ctx *)((void *)tctx + sizeof(*tctx) + tctx->ctx_len);
>> +}
>> +
>> +static inline void read_proc_attr(const char *attr, char *val, const size_t size)
>> +{
>> +	int fd;
>> +	char *ptr;
>> +	char path[BUFSIZ];
>> +
>> +	memset(val, 0, size);
>> +	memset(path, 0, BUFSIZ);
>> +
>> +	snprintf(path, BUFSIZ, "/proc/self/attr/%s", attr);
>> +
>> +	tst_res(TINFO, "Reading %s", path);
>> +
>> +	fd = SAFE_OPEN(path, O_RDONLY);
>> +
>> +	if (read(fd, val, size) > 0) {
>> +		ptr = strchr(val, '\n');
>> +		if (ptr)
>> +			*ptr = '\0';
>> +	}
>> +
>> +	SAFE_CLOSE(fd);
>> +}
>> +
>> +static inline int verify_enabled_lsm(const char *name)
>> +{
>> +	int fd;
>> +	char data[BUFSIZ];
>> +
>> +	fd = SAFE_OPEN("/sys/kernel/security/lsm", O_RDONLY);
>> +	SAFE_READ(0, fd, data, BUFSIZ);
>> +	SAFE_CLOSE(fd);
>> +
>> +	if (!strstr(data, name)) {
>> +		tst_res(TINFO, "%s is running", name);
>> +		return 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +#endif
>> diff --git a/testcases/kernel/syscalls/lsm/lsm_get_self_attr01.c b/testcases/kernel/syscalls/lsm/lsm_get_self_attr01.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2317941af1b73240368820e6a51591e7c18cc140
>> --- /dev/null
>> +++ b/testcases/kernel/syscalls/lsm/lsm_get_self_attr01.c
>> @@ -0,0 +1,81 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
>> + */
>> +
>> +/*\
>> + * [Description]
>> + *
>> + * Verify that lsm_get_self_attr syscall is raising errors when invalid data is
>> + * provided.
>> + */
>> +
>> +#include "tst_test.h"
>> +#include "lapi/lsm.h"
>> +
>> +static struct lsm_ctx *ctx;
>> +static uint32_t ctx_size;
>> +static uint32_t ctx_size_small;
>> +
>> +static struct tcase {
>> +	uint32_t attr;
> .attr = LSM_ATTR_CURRENT is the same for all 4 testcases.
> Can you please remove it from the test struct and use it directly?
I will keep it in order to test EOPNOTSUPP for overset attr flag, I will 
add this test case in the next version of the patchset.
>
>> +	struct lsm_ctx **ctx;
> The same applies to ctx.
>
> Also, kselftest test tools/testing/selftests/lsm/lsm_get_self_attr_test.c is
This is tested by lsm_get_self_attr02.
> testing also for ctx being NULL. Then it would make sense to use it.
> You would then need to use verify_enabled_lsm(), which you added in this commit
> but not use it (e.g. lsm_get_self_attr_test.c in kselftest checks for values
> when no lsm is set). Obviously you would have to store also errno for the case
> when lsm is not stored.
>
> On some Tumbleweed VM (6.10.0-rc7) I have the default:
>
> $ cat /sys/kernel/security/lsm
> lockdown,capability,landlock,yama,apparmor,bpf,ima,evm
>
> When I boot with lsm= kernel parameter, I get:
>
> $ cat /sys/kernel/security/lsm
> lockdown,capability,ima,evm
>
> And with that test fails:
>
> # ./lsm_get_self_attr01
> tst_buffers.c:57: TINFO: Test is using guarded buffers
> tst_test.c:1893: TINFO: LTP version: 20240930-146-gccd20cd77
> tst_test.c:1897: TINFO: Tested kernel: 6.10.0-rc7-3.g92abc10-default #1 SMP PREEMPT_DYNAMIC Wed Jul 10 14:15:11 UTC 2024 (92abc10) x86_64
> tst_test.c:1728: TINFO: Timeout per run is 0h 00m 30s
> lsm_get_self_attr01.c:67: TPASS: size is NULL : EINVAL (22)
> lsm_get_self_attr01.c:67: TPASS: flags is invalid : EINVAL (22)
> lsm_get_self_attr01.c:67: TFAIL: size is too smal expected E2BIG: EOPNOTSUPP (95)
> lsm_get_self_attr01.c:67: TPASS: flags force to use ctx attributes : EINVAL (22)
>
> => I would vote for having 2 variants to use EOPNOTSUPP or at least check with
> verify_enabled_lsm() and TCONF. Otherwise sooner or later somebody report a bug
> in the test.

Yeah, probably this is the best approach. I will add a helper checking 
for current LSM and eventually TCONF if they are not present in 
combination with LSM_ATTR_CURRENT.

Andrea