Re: [LTP] [PATCH v2] syscalls/set_thread_area01: Refactor into new API

[Petr Vorel <pvorel@suse.cz>]

Hi Ricardo,

> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
> ---
> Changes in v2:
> - Used tst_test.test_all instead of .test
> - Removed VALUE_AND_STRING macro
> - Added local wrappers to the syscalls
> - Fixed the test description
> - Added a comment quote from the set_thread_area manual to add context
> - Link to v1: https://lore.kernel.org/all/20250327-conversions-modify_ldt-v2-6-2907d4d3f6c0@suse.com
From Message-Id: generated by b4 it's obvious it is v2, not v1. I also tried to
find v1, but IMHO you haven't sent it.

> ---
>  .../syscalls/set_thread_area/set_thread_area.h     |  31 -----
>  .../syscalls/set_thread_area/set_thread_area01.c   | 133 +++++++--------------
>  2 files changed, 43 insertions(+), 121 deletions(-)

...
> +++ b/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
...
> +#ifdef __i386__

> -#if defined(HAVE_ASM_LDT_H) && defined(HAVE_STRUCT_USER_DESC)
BTW original test had #if defined(HAVE_ASM_LDT_H) && defined(HAVE_STRUCT_USER_DESC),
suggesting it was not i386 specific.

AC_CHECK_TYPES([struct user_desc, struct modify_ldt_ldt_s],[],[],[#include <asm/ldt.h>])

And there is fallback for struct modify_ldt_ldt_t if neither struct user_desc
nor struct modify_ldt_ldt_s is available. That's also suggest other archs.

I was able to test it only on x86_64, where it works only when 32 bit
compatibility is set. That indeed works for #ifdef __i386__ or it could be guarded by:

.supported_archs = (const char *const []){"x86", NULL},
This way it would appear in the test catalog doc.

Trying to run it on regular x86_64, it TCONF due tst_syscalls() guard:

set_thread_area01.c:39: TCONF: syscall(205) __NR_set_thread_area not supported on your arch

Which is strange, because I see in arch/x86/kernel/tls.c in do_set_thread_area()
(the implementation) part of the code with #ifdef CONFIG_X86_64.

If it's really for 32 bit (I don't think so) but supported on more archs, it
could be guarded by .needs_abi_bits = 32 (again metadata doc).

Just investigating, looking at man set_thread_area(2) it mentions:

	At the moment, set_thread_area() is
	available on m68k, MIPS, C-SKY, and x86 (both 32-bit and 64-bit
	variants); get_thread_area() is available on m68k and x86.

[2] https://man7.org/linux/man-pages/man2/set_thread_area.2.html

Looking at kernel code it also shows that other archs are included,
IMHO the relevant nowadays (besides x86*) are only e.g. arm, arm64/aarch64.

$ git grep -l set_thread_area arch/
arch/arm/tools/syscall.tbl
arch/arm64/tools/syscall_32.tbl
arch/csky/include/asm/syscalls.h
arch/csky/kernel/syscall.c
arch/m68k/include/asm/syscalls.h
arch/m68k/kernel/sys_m68k.c
arch/m68k/kernel/syscalls/syscall.tbl
arch/microblaze/kernel/syscalls/syscall.tbl
arch/mips/kernel/syscall.c
arch/mips/kernel/syscalls/syscall_n32.tbl
arch/mips/kernel/syscalls/syscall_n64.tbl
arch/mips/kernel/syscalls/syscall_o32.tbl
arch/parisc/kernel/syscalls/syscall.tbl
arch/sh/kernel/syscalls/syscall.tbl
arch/um/kernel/ptrace.c
arch/x86/entry/syscalls/syscall_32.tbl
arch/x86/entry/syscalls/syscall_64.tbl
arch/x86/include/asm/ptrace.h
arch/x86/include/uapi/asm/sigcontext.h
arch/x86/kernel/process.c
arch/x86/kernel/ptrace.c
arch/x86/kernel/tls.c
arch/x86/um/asm/ptrace.h
arch/x86/um/os-Linux/tls.c
arch/x86/um/shared/sysdep/tls.h
arch/x86/um/tls_32.c
arch/xtensa/kernel/syscalls/syscall.tbl

But that means nothing. Checking at a real set_thread_area() definition I see:

$ git grep -l 'SYSCALL_DEFINE.*(set_thread_area' arch/
arch/csky/kernel/syscall.c
arch/mips/kernel/syscall.c
arch/x86/kernel/tls.c
arch/x86/um/tls_32.c

=> it probably does not work on arm and arm64/aarch64.

But maybe just not specify anything? Because tst_syscall() will quit with TCONF
in case kernel does not support it? That case we don't block anybody from mips,
csky folks to do testing if they want. But...

> +#include "lapi/ldt.h"

... given it requires 'struct user_desc' from <asm/ldt.h>, which is only for
x86/i386 and x86_64, I would chose:

.supported_archs = (const char *const []){"x86", NULL},

Ah, the dependency [1] is at the end in b4 tag:
> prerequisite-message-id: <20250402-conversions-modify_ldt-v6-0-2e4b0e27870e@suse.com>

Later I'll check, because it's obvious that it needs include/lapi/ldt.h from
another patchset, but I was not sure whether v6 should be used (anyway, it's the
newest). For me it'd be quickest if I could just see working branch on the
remote fork (+ have CI run for free). BTW if we one day implement automatic CI
for sending patches, it will not work if some of depending commits is separated.

[1] https://patchwork.ozlabs.org/project/ltp/patch/20250402-conversions-modify_ldt-v6-1-2e4b0e27870e@suse.com/

> +/* When set_thread_area() is passed an entry_number of -1, it  searches
> + * for a free TLS entry. If set_thread_area() finds a free TLS entry,
> + * the value of u_info->entry_number is set upon return to show which
> + * entry was changed.
> + */

...
> +void run(void)
>  {
> -	int lc;
> -	unsigned i;
> -
> -	tst_parse_opts(argc, argv, NULL, NULL);
> -
> -	setup();
> -
> -	for (lc = 0; TEST_LOOPING(lc); lc++) {
> -		for (i = 0; i < sizeof(tests) / sizeof(struct test); i++) {
> -			TEST(tst_syscall(tests[i].syscall, tests[i].u_info));
> +	TST_EXP_PASS(set_thread_area(&entry));
> +	TST_EXP_PASS(get_thread_area(&entry));

> -			if (TEST_RETURN != tests[i].exp_ret) {
> -				tst_resm(TFAIL, "%s returned %li expected %i",
> -					 tests[i].syscall_name,
> -					 TEST_RETURN, tests[i].exp_ret);
> -				continue;
> -			}
> +	TST_EXP_FAIL(set_thread_area(&invalid_entry), EINVAL);
> +	TST_EXP_FAIL(get_thread_area(&invalid_entry), EINVAL);

> -			if (TEST_ERRNO != tests[i].exp_errno) {
> -				tst_resm(TFAIL,
> -					 "%s failed with %i (%s) expected %i (%s)",
> -					 tests[i].syscall_name, TEST_ERRNO,
> -					 strerror(TEST_ERRNO),
> -					 tests[i].exp_errno,
> -					 strerror(tests[i].exp_errno));
> -				continue;
> -			}
> +	TST_EXP_FAIL(set_thread_area((void *)-9), EFAULT);
> +	TST_EXP_FAIL(get_thread_area((void *)-9), EFAULT);

This will be a problem for portablity outside of i386 (x86).
We have tst_get_bad_addr(NULL), and it works for me on x86_64 with 32 bit
compatibility.

Kind regards,
Petr

> ---
> base-commit: 898cc14ad412fb521867b43fed5c4e067b76f809
> change-id: 20250403-conversions-set_thread_area-07a90e0cd449
> prerequisite-message-id: <20250402-conversions-modify_ldt-v6-0-2e4b0e27870e@suse.com>
> prerequisite-patch-id: 490a3e6bc4004db5234224b6fd6d4bf5030b219d
> prerequisite-patch-id: 962bb815444eb2de9756dd2659e097567b6d6fe8
> prerequisite-patch-id: a8357fb870a8d7278e206b4fc65f1d9450fee802

> Best regards,

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