[LTP] [PATCH v4 2/3] syscalls/modify_ldt02: Refactor into new API
Andrea Cervesato
andrea.cervesato@suse.com
Mon Mar 31 10:23:12 CEST 2025
Hi!
On 3/28/25 20:33, Ricardo B. Marliere via ltp wrote:
> From: Ricardo B. Marlière<rbm@suse.com>
>
> Signed-off-by: Ricardo B. Marlière<rbm@suse.com>
> ---
> testcases/kernel/syscalls/modify_ldt/common.h | 30 +++
> .../kernel/syscalls/modify_ldt/modify_ldt02.c | 222 +++++----------------
> 2 files changed, 77 insertions(+), 175 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/modify_ldt/common.h b/testcases/kernel/syscalls/modify_ldt/common.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..40e33251528dc3352c39c221f26bb91733424162
> --- /dev/null
> +++ b/testcases/kernel/syscalls/modify_ldt/common.h
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) International Business Machines Corp., 2001
> + * 07/2001 Ported by Wayne Boyer
> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière<rbm@suse.com>
> + */
> +
> +#ifndef COMMON_H
> +#define COMMON_H
> +
> +#include "tst_test.h"
> +#include "lapi/ldt.h"
> +
> +int create_segment(void *seg, size_t size)
> +{
> + struct user_desc entry = {
> + .entry_number = 0,
> + .base_addr = (unsigned long)seg,
> + .limit = size,
> + .seg_32bit = 1,
> + .contents = 0,
> + .read_exec_only = 0,
> + .limit_in_pages = 0,
> + .seg_not_present = 0,
> + };
> +
> + return SAFE_MODIFY_LDT(1, &entry, sizeof(entry));
> +}
> +
> +#endif
> diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c b/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
> index 5113b3fb4e29d241e819e98c90c297107aec93cf..600659d4495b77c5fb920ecbec3880e95b6d6601 100644
> --- a/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
> +++ b/testcases/kernel/syscalls/modify_ldt/modify_ldt02.c
> @@ -1,199 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> /*
> - *
> - * Copyright (c) International Business Machines Corp., 2001
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See
> - * the GNU General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> - */
> -
> -/*
> - * NAME
> - * modify_ldt02.c
> - *
> - * DESCRIPTION
> - * Testcase to check the error conditions for modify_ldt(2)
> - *
> - * ALGORITHM
> - * block1:
> - * Create a segment at entry 0 and a valid base address.
> - * Read the contents of the segment thru' fs register.
> - * Validate the data.
> - * Write an invalid base address into entry 0.
> - * Read the contents of entry 0 in the child process.
> - * Verify that a SIGSEGV is incurred.
> - *
> - * USAGE
> - * modify_ldt02
> - *
> - * HISTORY
> + * Copyright (c) International Business Machines Corp., 2001
> * 07/2001 Ported by Wayne Boyer
> - *
> - * RESTRICTIONS
> - * None
> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière<rbm@suse.com>
> */
>
> -#include "config.h"
> -#include "test.h"
> -
> -TCID_DEFINE(modify_ldt02);
> -int TST_TOTAL = 1;
> -
> -#if defined(__i386__) && defined(HAVE_MODIFY_LDT)
> -
> -#ifdef HAVE_ASM_LDT_H
> -#include <asm/ldt.h>
> -#endif
> -extern int modify_ldt(int, void *, unsigned long);
> -
> -#include <asm/unistd.h>
> -#include <string.h>
> -#include <sys/wait.h>
> -#include <errno.h>
> -
> -/* Newer ldt.h files use user_desc, instead of modify_ldt_ldt_s */
> -#ifdef HAVE_STRUCT_USER_DESC
> -typedef struct user_desc modify_ldt_s;
> -#elif HAVE_STRUCT_MODIFY_LDT_LDT_S
> -typedef struct modify_ldt_ldt_s modify_ldt_s;
> -#else
> -typedef struct modify_ldt_ldt_t {
> - 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;
> -} modify_ldt_s;
> -#endif
> -
> -int create_segment(void *, size_t);
> -int read_segment(unsigned int);
> -void cleanup(void);
> -void setup(void);
> -
> -int main(int ac, char **av)
> -{
> - int lc;
> -
> - int val, pid, status;
val is never used inside parent, so we should define it inside child in
order to avoid any warning.
> -
> - int seg[4];
> -
> - tst_parse_opts(ac, av, NULL, NULL);
> -
> - setup();
> -
> - for (lc = 0; TEST_LOOPING(lc); lc++) {
> - tst_count = 0;
> -
> - seg[0] = 12345;
> - if (create_segment(seg, sizeof(seg)) == -1) {
> - tst_brkm(TBROK, cleanup, "Creation of segment failed");
> - }
> -
> - val = read_segment(0);
> -
> - if (val != seg[0]) {
> - tst_resm(TFAIL, "Invalid value read %d, expected %d",
> - val, seg[0]);
> - } else
> - tst_resm(TPASS, "value read as expected");
> -
> - if (create_segment(0, 10) == -1) {
> - tst_brkm(TBROK, cleanup, "Creation of segment failed");
> - }
> -
> - tst_old_flush();
> - if ((pid = tst_fork()) == 0) {
> - signal(SIGSEGV, SIG_DFL);
> - val = read_segment(0);
> - exit(1);
> - }
> -
> - (void)waitpid(pid, &status, 0);
> -
> - if (WEXITSTATUS(status) != 0) {
> - tst_resm(TFAIL, "Did not generate SEGV, child returned "
> - "unexpected status");
> - } else {
> - if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV))
> - tst_resm(TPASS, "generate SEGV as expected");
> - else
> - tst_resm(TFAIL, "Did not generate SEGV");
> - }
> - }
> - cleanup();
> - tst_exit();
> -
> -}
> -
> -int create_segment(void *seg, size_t size)
> -{
> - modify_ldt_s entry;
> +/*\
> + * Verify that after an invalid write to the entry, a subsequent read will
> + * raise a SIGSEV.
Maybe we can improve a bit the description:
Verify that after writing an invalid base address into a segment entry,
a subsequent segment entry read will raise SIGSEV.
> + */
>
> - entry.entry_number = 0;
> - entry.base_addr = (unsigned long)seg;
> - entry.limit = size;
> - entry.seg_32bit = 1;
> - entry.contents = 0;
> - entry.read_exec_only = 0;
> - entry.limit_in_pages = 0;
> - entry.seg_not_present = 0;
> +#include "tst_test.h"
>
> - return modify_ldt(1, &entry, sizeof(entry));
> -}
> +#ifdef __i386__
> +#include "lapi/ldt.h"
This import is not needed since we have it already inside common.h.
> +#include "common.h"
>
> int read_segment(unsigned int index)
> {
> int res;
> +
> __asm__ __volatile__("\n\
> push $0x0007;\n\
> pop %%fs;\n\
> - movl %%fs:(%1), %0":"=r"(res)
> - :"r"(index * sizeof(int)));
> + movl %%fs:(%1), %0"
> + : "=r"(res)
> + : "r"(index * sizeof(int)));
> +
> return res;
> }
>
> -void setup(void)
> +void run(void)
> {
> - tst_sig(FORK, DEF_HANDLER, cleanup);
> + int val, pid, status, seg[4];
>
> - TEST_PAUSE;
> -}
> + seg[0] = 12345;
>
> -void cleanup(void)
> -{
> + create_segment(seg, sizeof(seg));
>
> -}
> -#elif HAVE_MODIFY_LDT
> -int main(void)
> -{
> - tst_brkm(TCONF,
> - NULL,
> - "modify_ldt is available but not tested on the platform than __i386__");
> -}
> + TST_EXP_EQ_LI(read_segment(0), seg[0]);
>
> -#else /* if defined(__i386__) */
> + create_segment(0, 10);
>
> -int main(void)
> -{
> - tst_resm(TINFO, "modify_ldt02 test only for ix86");
> - tst_exit();
> + pid = SAFE_FORK();
> + if (!pid) {
> + signal(SIGSEGV, SIG_DFL);
> + val = read_segment(0);
> + exit(1);
> + }
> +
> + SAFE_WAITPID(pid, &status, 0);
> + if (WEXITSTATUS(status) != 0) {
> + tst_res(TFAIL, "Did not generate SEGV, child returned "
> + "unexpected status");
> + } else {
> + if (WIFSIGNALED(status) && (WTERMSIG(status) == SIGSEGV))
> + tst_res(TPASS, "generate SEGV as expected");
> + else
> + tst_res(TFAIL, "Did not generate SEGV");
> + }
> }
>
> -#endif /* if defined(__i386__) */
> +static struct tst_test test = {
> + .test_all = run,
> + .forks_child = 1,
> +};
> +
> +#else
> +TST_TEST_TCONF("Test supported only on i386");
> +#endif /* __i386__ */
>
Otherwise LGTM
Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
Kind regards,
Andrea Cervesato
More information about the ltp
mailing list