[LTP] [PATCH v2 1/6] syscalls/modify_ldt: Add lapi/ldt.h
Andrea Cervesato
andrea.cervesato@suse.com
Fri Mar 28 10:15:56 CET 2025
Hi Ricardo,
On 3/27/25 15:28, Ricardo B. Marliere via ltp wrote:
> From: Ricardo B. Marlière <rbm@suse.com>
>
> Add a wrapper to modify_ldt and a fallback to the user_desc structure which
> is used in a few tests and should be reused.
>
> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
> ---
> configure.ac | 1 -
> include/lapi/ldt.h | 61 +++++++++++++++++++++++++++++++++
> testcases/cve/cve-2015-3290.c | 35 ++++---------------
> testcases/cve/cve-2017-17053.c | 10 +++---
> testcases/kernel/syscalls/fork/fork05.c | 5 ++-
> 5 files changed, 74 insertions(+), 38 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index 671079f3aa48c72866f69e7c545d3428ba87f931..b22f3eacdb1ccb764eae443ab1fb70afd971a14c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -46,7 +46,6 @@ AC_CHECK_DECLS([PR_CAPBSET_DROP, PR_CAPBSET_READ],,,[#include <sys/prctl.h>])
> AC_CHECK_DECLS([SEM_STAT_ANY],,,[#include <sys/sem.h>])
>
> AC_CHECK_HEADERS_ONCE([ \
> - asm/ldt.h \
> asm/prctl.h \
> cpuid.h \
> emmintrin.h \
> diff --git a/include/lapi/ldt.h b/include/lapi/ldt.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..05def195bd84b9bad4725525e9fede38df5ae7be
> --- /dev/null
> +++ b/include/lapi/ldt.h
> @@ -0,0 +1,61 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
> + */
> +
> +#if !(defined(__i386__) || defined(__x86_64__))
> +#error "ldt.h should only be included on x86"
> +#endif
> +
> +#ifndef LAPI_LDT_H__
> +#define LAPI_LDT_H__
> +
> +#include "config.h"
> +#include "lapi/syscalls.h"
> +
> +#ifdef HAVE_STRUCT_USER_DESC
> +#include <asm/ldt.h>
> +#else
> +struct user_desc {
> + unsigned int entry_number;
> + unsigned int base_addr;
> + unsigned int limit;
> + unsigned int seg_32bit : 1;
> + unsigned int contents : 2;
> + unsigned int read_exec_only : 1;
> + unsigned int limit_in_pages : 1;
> + unsigned int seg_not_present : 1;
> + unsigned int useable : 1;
> +#ifdef __x86_64__
> + unsigned int lm : 1;
> +#endif /* __x86_64__ */
> +};
> +#endif /* HAVE_STRUCT_USER_DESC */
> +
> +#ifdef HAVE_STRUCT_MODIFY_LDT_LDT_S
> +/* In Linux 2.4 and earlier, this structure was named modify_ldt_ldt_s. */
> +typedef struct modify_ldt_ldt_s user_desc;
> +#endif /* HAVE_STRUCT_MODIFY_LDT_LDT_S */
All the above is not needed, because we always support asm/ldt.h
functionalities. The reason is that our minimum supported kernel is 4.4,
while feature was still present in 2.x.
You can safely remove HAVE_STRUCT_USER_DESC and the fallback definitions
of user_desc.
> +
> +static int modify_ldt(int func, void *ptr, unsigned long bytecount)
> +{
> + return tst_syscall(__NR_modify_ldt, func, ptr, bytecount);
> +}
> +
> +static int safe_modify_ldt(const char *file, const int lineno, int func,
> + void *ptr, unsigned long bytecount)
> +{
> + int rval;
> +
> + rval = modify_ldt(func, ptr, bytecount);
> + if (rval == -1)
> + tst_brk_(file, lineno, TBROK | TERRNO,
> + "modify_ldt(%d, %p, %lu)", func, ptr, bytecount);
> +
> + return rval;
> +}
> +
> +#define SAFE_MODIFY_LDT(func, ptr, bytecount) \
> + safe_modify_ldt(__FILE__, __LINE__, (func), (ptr), (bytecount))
> +
> +#endif /* LAPI_LDT_H__ */
> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
> index 63e5d92c91b830cd8066a6a6c329461b72731f32..d1bd1d9415359d2568c6694f15ffe8afe2a1f690 100644
> --- a/testcases/cve/cve-2015-3290.c
> +++ b/testcases/cve/cve-2015-3290.c
> @@ -118,12 +118,10 @@ perhaps unsurprisingly.)
> #include "tst_test.h"
> #include "tst_timer.h"
>
> -#if defined(__x86_64__) || defined(__i386__)
> -
> +#if defined(__i386__) || defined(__x86_64__)
> #include <stdlib.h>
> #include <stdio.h>
> #include <inttypes.h>
> -#include <asm/ldt.h>
> #include <unistd.h>
> #include <sys/syscall.h>
> #include <setjmp.h>
> @@ -132,6 +130,7 @@ perhaps unsurprisingly.)
> #include <sys/wait.h>
> #include <linux/perf_event.h>
>
> +#include "lapi/ldt.h"
> #include "lapi/syscalls.h"
> #include "tst_safe_pthread.h"
>
> @@ -199,27 +198,7 @@ static void set_ldt(void)
> .useable = 0
> };
>
> - TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
> -
> - /*
> - * The kernel intentionally casts modify_ldt() return value
> - * to unsigned int to prevent sign extension to 64 bits. This may
> - * result in syscall() returning the value as is instead of setting
> - * errno and returning -1.
> - */
> - if (TST_RET > 0 && ((int)TST_RET) < 0) {
> - tst_res(TINFO,
> - "WARNING: Libc mishandled modify_ldt() return value");
> - TST_ERR = -(int)TST_RET;
> - TST_RET = -1;
> - }
Sorry but I missed this. It seems to be right...kernel code in
arch/x86/kernel/ldt.c:687
Even kselftests are not taking care of this and we should follow
documentation, which says -1 return value is the default value for error.
I have the feeling this code was wrong. @Cyril what do you think?
> -
> - if (TST_RET == -1 && TST_ERR == EINVAL) {
> - tst_brk(TCONF | TTERRNO,
> - "modify_ldt: 16-bit data segments are probably disabled");
> - } else if (TST_RET != 0) {
> - tst_brk(TBROK | TTERRNO, "modify_ldt");
> - }
> + SAFE_MODIFY_LDT(1, &data_desc, sizeof(data_desc));
> }
>
> static void try_corrupt_stack(unsigned short *orig_ss)
> @@ -528,8 +507,6 @@ static struct tst_test test = {
> }
> };
>
> -#else /* defined(__x86_64__) || defined(__i386__) */
> -
> -TST_TEST_TCONF("not (i386 or x86_64)");
> -
> -#endif
> +#else
> +TST_TEST_TCONF("Test supported only on x86");
> +#endif /* defined(__i386__) || defined(__x86_64__) */
> diff --git a/testcases/cve/cve-2017-17053.c b/testcases/cve/cve-2017-17053.c
> index fe7b6d694d6ffbbce863abc1672e03ae5f419df1..7ba22fa94cee57fcb0b0b60c2462d036cb4a40c5 100644
> --- a/testcases/cve/cve-2017-17053.c
> +++ b/testcases/cve/cve-2017-17053.c
> @@ -16,8 +16,7 @@
> #include "config.h"
> #include "tst_test.h"
>
> -#ifdef HAVE_ASM_LDT_H
> -#include <asm/ldt.h>
> +#if defined(__i386__) || defined(__x86_64__)
> #include <pthread.h>
> #include <signal.h>
> #include <stdlib.h>
> @@ -26,6 +25,7 @@
> #include <unistd.h>
> #include <stdio.h>
>
> +#include "lapi/ldt.h"
> #include "lapi/syscalls.h"
>
> #define EXEC_USEC 5000000
> @@ -109,7 +109,7 @@ void run_test(void)
> struct user_desc desc = { .entry_number = 8191 };
>
> install_sighandler();
> - syscall(__NR_modify_ldt, 1, &desc, sizeof(desc));
> + SAFE_MODIFY_LDT(1, &desc, sizeof(desc));
>
> for (;;) {
> if (shm->do_exit)
> @@ -164,5 +164,5 @@ static struct tst_test test = {
> };
>
> #else
> -TST_TEST_TCONF("no asm/ldt.h header (only for i386 or x86_64)");
> -#endif
> +TST_TEST_TCONF("Test supported only on x86");
> +#endif /* defined(__i386__) || defined(__x86_64__) */
> diff --git a/testcases/kernel/syscalls/fork/fork05.c b/testcases/kernel/syscalls/fork/fork05.c
> index 22edefc3686978fbb9453dffabfcbccb7ea6bb12..9aa12e16201dec8f3d2a4c99df83c4e5e25ef857 100644
> --- a/testcases/kernel/syscalls/fork/fork05.c
> +++ b/testcases/kernel/syscalls/fork/fork05.c
> @@ -55,8 +55,7 @@
>
> #if defined(__i386__)
>
> -#include "lapi/syscalls.h"
> -#include <asm/ldt.h>
> +#include "lapi/ldt.h"
>
> static void run(void)
> {
> @@ -76,7 +75,7 @@ static void run(void)
> ldt0.seg_not_present = 0;
> ldt0.useable = 1;
>
> - tst_syscall(__NR_modify_ldt, 1, &ldt0, sizeof(ldt0));
> + SAFE_MODIFY_LDT(1, &ldt0, sizeof(ldt0));
>
> asm volatile ("movw %w0, %%fs"::"q" (7));
> asm volatile ("movl %%fs:0, %0":"=r" (lo));
>
The rest looks ok,
Andrea
More information about the ltp
mailing list