[LTP] [PATCH 1/4] modify_ldt: Add lapi/ldt.h
Ricardo B. Marlière
rbm@suse.com
Tue Mar 25 13:34:19 CET 2025
On Tue Mar 25, 2025 at 6:58 AM -03, Andrea Cervesato wrote:
> Hi!
>
> The modify_ldt syscall exists for a long time before the minimum kernel
> version 4.4 that we support in LTP.
> So we can happily ignore any fallback definition and to remove asm/ldt.h
> check from configure.ac.
Pardon my ignorance, but if we remove this line:
https://github.com/linux-test-project/ltp/blob/ae2845368acd79650cc483a9857b92725c0174d0/configure.ac#L49
Would we need to find and fix all occurrences of HAVE_ASM_LDT_H ? The
build system is still a bit mysterious to me...
>
> More comments below.
>
> On 3/24/25 21:45, 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>
>> ---
>> include/lapi/ldt.h | 53 ++++++++++++++++++++++
>> testcases/cve/cve-2015-3290.c | 15 +++---
>> testcases/cve/cve-2017-17053.c | 10 ++--
>> testcases/kernel/syscalls/fork/fork05.c | 4 +-
>> .../syscalls/set_thread_area/set_thread_area.h | 31 -------------
>> .../syscalls/set_thread_area/set_thread_area01.c | 14 +++---
>> 6 files changed, 74 insertions(+), 53 deletions(-)
>>
>> diff --git a/include/lapi/ldt.h b/include/lapi/ldt.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..011138a1d22adfd94bcda9b31cafd3746272556b
>> --- /dev/null
>> +++ b/include/lapi/ldt.h
>> @@ -0,0 +1,53 @@
>> +// 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_ASM_LDT_H
>> +#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_ASM_LDT_H */
>> +
>> +#ifdef HAVE_STRUCT_MODIFY_LDT_LDT_S
>> +/*
>> + * From man 2 modify_ldt:
>> + * 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 */
>> +
>> +/*
>> + * From man 2 modify_ldt:
>> + * Note: glibc provides no wrapper for modify_ldt(), necessitating the use of
>> + * syscall(2).
>> + */
>> +static int modify_ldt(int func, void *ptr, unsigned long bytecount)
>> +{
>> + return syscall(__NR_modify_ldt, func, ptr, bytecount);
> If we want to define any wrapper around a syscall, we need to use
> tst_syscall. It's a safer way to handle syscalls errors due to ENOSYS.
>
> We are also checking for modify_ldt(2) syscall in the configure.ac file,
> so we can use HAVE_MODIFY_LDT to verify that no wrappers have been
> defined before.
>
> Then we can add a SAFE_MODIFY_LDT() macro in order to use syscall is a
> safe way.
>
That a great idea! I will try in v2 :)
First I tried using tst_syscall but then it broke some functionality of
the existing tests. I'll review those in v2...
>> +}
>> +
>> +#endif /* LAPI_LDT_H__ */
>> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
>> index 63e5d92c91b830cd8066a6a6c329461b72731f32..ce90c90942e5780d8f86adc5bd18718a96c1a95e 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,7 +198,7 @@ static void set_ldt(void)
>> .useable = 0
>> };
>>
>> - TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
>> + TEST(modify_ldt(1, &data_desc, sizeof(data_desc)));
> This test can use SAFE_MODIF_LDT
>>
>> /*
>> * The kernel intentionally casts modify_ldt() return value
>> @@ -528,8 +527,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..1258548de8f6803f347aa6d19dcbaeacaffec30d 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));
>> + modify_ldt(1, &desc, sizeof(desc));
> This test can use SAFE_MODIF_LDT
>>
>> 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..1223aaff7d38152d5116b9011a34ff4c762cc987 100644
>> --- a/testcases/kernel/syscalls/fork/fork05.c
>> +++ b/testcases/kernel/syscalls/fork/fork05.c
>> @@ -55,8 +55,8 @@
>>
>> #if defined(__i386__)
>>
>> +#include "lapi/ldt.h"
>> #include "lapi/syscalls.h"
>> -#include <asm/ldt.h>
>>
>> static void run(void)
>> {
>> @@ -76,7 +76,7 @@ static void run(void)
>> ldt0.seg_not_present = 0;
>> ldt0.useable = 1;
>>
>> - tst_syscall(__NR_modify_ldt, 1, &ldt0, sizeof(ldt0));
>> + modify_ldt(1, &ldt0, sizeof(ldt0));
>>
> This test can use SAFE_MODIF_LDT
>> asm volatile ("movw %w0, %%fs"::"q" (7));
>> asm volatile ("movl %%fs:0, %0":"=r" (lo));
>> diff --git a/testcases/kernel/syscalls/set_thread_area/set_thread_area.h b/testcases/kernel/syscalls/set_thread_area/set_thread_area.h
>> deleted file mode 100644
>> index 2bd2469d549ecf8c5c589a0a9485f886d043f7ed..0000000000000000000000000000000000000000
>> --- a/testcases/kernel/syscalls/set_thread_area/set_thread_area.h
>> +++ /dev/null
>> @@ -1,31 +0,0 @@
>> -#include <stdio.h>
>> -#include <errno.h>
>> -
>> -/* Harness Specific Include Files. */
>> -#include "test.h"
>> -#include "lapi/syscalls.h"
>> -#include "config.h"
>> -
>> -#if defined HAVE_ASM_LDT_H
>> -#include <linux/unistd.h>
>> -#include <asm/ldt.h>
>> -
>> -#if defined HAVE_STRUCT_USER_DESC
>> -typedef struct user_desc thread_area_s;
>> -#elif defined HAVE_STRUCT_MODIFY_LDT_LDT_S
>> -typedef struct modify_ldt_ldt_s thread_area_s;
>> -#else
>> -typedef struct user_desc {
>> - unsigned int entry_number;
>> - unsigned long 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;
>> - unsigned int empty:25;
>> -} thread_area_s;
>> -#endif /* HAVE_STRUCT_USER_DESC */
>> -#endif /* HAVE_ASM_LDT_H */
>> diff --git a/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c b/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
>> index 30626d5e90ebf8e20624c370cc4474cb51a6b102..6c6fd522cb153acc3693718097a0c33a656ad747 100644
>> --- a/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
>> +++ b/testcases/kernel/syscalls/set_thread_area/set_thread_area01.c
>> @@ -19,13 +19,16 @@
>> *
>> ************************************************************************/
>>
>> -#include "set_thread_area.h"
>> +#include "config.h"
>> +#include "test.h"
>> +
>> +#if defined(__i386__) || defined(__x86_64__)
>> +#include "lapi/syscalls.h"
>> +#include "lapi/ldt.h"
>>
>> char *TCID = "set_thread_area_01";
>> int TST_TOTAL = 6;
>>
>> -#if defined(HAVE_ASM_LDT_H) && defined(HAVE_STRUCT_USER_DESC)
>> -
>> static void cleanup(void)
>> {
>> }
>> @@ -38,7 +41,7 @@ static void setup(void)
>> struct test {
>> int syscall;
>> const char *const syscall_name;
>> - thread_area_s *u_info;
>> + struct user_desc *u_info;
> In general we don't both too much with old tests, unless there's a big
> issue. I wouldn't to touch it at the moment, unless we want to refactor it.
Indeed, my goal is to take that next since it's a simple one.
>> int exp_ret;
>> int exp_errno;
>> };
>> @@ -105,7 +108,6 @@ int main(int argc, char *argv[])
>> #else
>> int main(void)
>> {
>> - tst_brkm(TCONF, NULL,
>> - "set_thread_area isn't available for this architecture");
>> + tst_brkm(TCONF, NULL, "Test supported only in x86");
>> }
>> #endif
>
> Kind regards,
> Andrea Cervesato
Thank you,
- Ricardo.
More information about the ltp
mailing list