[LTP] [PATCH v4 1/3] syscalls/modify_ldt: Add lapi/ldt.h
Ricardo B. Marlière
rbm@suse.com
Mon Mar 31 19:29:17 CEST 2025
Hello Andrea,
On Mon Mar 31, 2025 at 4:06 AM -03, Andrea Cervesato wrote:
> Hi!
>
> On 3/28/25 20:33, Ricardo B. Marliere via ltp wrote:
>> From: Ricardo B. Marlière <rbm@suse.com>
>>
>> Add a wrapper to modify_ldt 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 | 38 +++++++++++++++++++++++++++++++++
>> testcases/cve/cve-2015-3290.c | 35 ++++++------------------------
>> testcases/cve/cve-2017-17053.c | 10 ++++-----
>> testcases/kernel/syscalls/fork/fork05.c | 5 ++---
>> 5 files changed, 51 insertions(+), 38 deletions(-)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 411f8b4cae57e5872f33f7e327c180fbfb8e7d26..ae5d13422b4b8ce0e1e367fef92b693a1c6093a5 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..962ffa880fe6f41146c0bd790a55714d0313e20f
>> --- /dev/null
>> +++ b/include/lapi/ldt.h
>> @@ -0,0 +1,38 @@
>> +// 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
> I guess this check is not needed anymore because we are handling it
> inside the tests and eventually we obtain a ENOSYS while running
> modify_ldt syscall in the wrong place.
Ah, thanks for pointing this out. I borrowed it from include/lapi/cpuid.h
but I see now how its not applicable here!
Also, thank you for the reviews!
>> +
>> +#ifndef LAPI_LDT_H__
>> +#define LAPI_LDT_H__
>> +
>> +#include "config.h"
>> +#include "lapi/syscalls.h"
>> +#include <asm/ldt.h>
>> +
>> +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);
> We will probably need to handle the libc return value conversion here,
> but let's wait for @Cyril comment first.
I'll send a v5 anyway and we'll continue from there
>> +
>> + 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");
> ^ @Cyril
>> - TST_ERR = -(int)TST_RET;
>> - TST_RET = -1;
>> - }
>> -
>> - 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));
>>
> Otherwise LGTM
>
> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
>
> Kind regards,
> Andrea Cervesato
More information about the ltp
mailing list