[LTP] [PATCH v2 4/6] syscalls/modify_ldt01: Refactor into new API
Ricardo B. Marlière
rbm@suse.com
Fri Mar 28 10:47:35 CET 2025
On Fri Mar 28, 2025 at 6:29 AM -03, Andrea Cervesato wrote:
> Hi!
>
> This follow-up patch should be merged with modify_ldt01 refactoring patch.
> There's no need to create too many patches. To make it easier, we should
> have the following ones:
>
> - create fallback file with SAFE_* macro + SAFE_* macro usage inside
> other tests
> - refactor modify_ldt01 (including modify_ldt03 in it)
> - refactor modify_ldt02
> - refactor set_thread_area01
Works for me, will squash in v3.
>
> Some comments below.
>
> On 3/27/25 15:28, Ricardo B. Marliere via ltp wrote:
>> From: Ricardo B. Marlière <rbm@suse.com>
>>
>> Now that we're using the wrapper modify_ldt() from the lapi/ldt.h which
>> uses tst_syscall(), it does not make sense to keep the first block of the
>> original test.
>>
>> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
>> ---
>> .../kernel/syscalls/modify_ldt/modify_ldt01.c | 254 ++++-----------------
>> 1 file changed, 44 insertions(+), 210 deletions(-)
>>
>> diff --git a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
>> index 684e53772414ae468b4f168578596eabb27ef18b..d93a2815f980c17e9490d0285c63d76c69e88d02 100644
>> --- a/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
>> +++ b/testcases/kernel/syscalls/modify_ldt/modify_ldt01.c
>> @@ -1,230 +1,64 @@
>> +// 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
>> + * 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>
>> */
>>
>> -/*
>> - * NAME
>> - * modify_ldt01.c
>> - *
>> - * DESCRIPTION
>> - * Testcase to check the error conditions for modify_ldt(2)
>> +/*\
>> + * Verify that modify_ldt() call fails with errno:
>> *
>> - * CALLS
>> - * modify_ldt()
>> - *
>> - * ALGORITHM
>> - * block1:
>> - * Invoke modify_ldt() with a func value which is neither
>> - * 0 or 1. Verify that ENOSYS is set.
>> - * block2:
>> - * Invoke mprotect() with ptr == NULL. Verify that EINVAL
>> - * is set.
>> - * block3:
>> - * Create an LDT segment.
>> - * Try to read from an invalid pointer.
>> - * Verify that EFAULT is set.
>> - *
>> - * USAGE
>> - * modify_ldt01
>> - *
>> - * HISTORY
>> - * 07/2001 Ported by Wayne Boyer
>> - *
>> - * RESTRICTIONS
>> - * None
>> + * - EINVAL, when writing (func=1) to an invalid pointer
>> + * - EFAULT, when reading (func=0) from an invalid pointer
>> */
>>
>> -#include "config.h"
>> -#include "test.h"
>> -
>> -TCID_DEFINE(modify_ldt01);
>> -int TST_TOTAL = 1;
>> +#include "tst_test.h"
>>
>> -#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 <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);
>> -void cleanup(void);
>> -void setup(void);
>> -
>> -int main(int ac, char **av)
>> -{
>> - int lc;
>> +#ifdef __i386__
>> +#include "lapi/ldt.h"
>> +#include "common.h"
>>
>> +static void *ptr;
>> +static char *buf;
>> +static struct tst_case {
> tst_* is a reserved prefix for LTP library, so it's important to use
> something different. We usually name it "tcase"
Ah, makes sense, I didn't know that!
Thanks for reviewing,
- Ricardo.
>> + int tfunc;
>> void *ptr;
>> - int retval, func;
>> -
>> - int seg[4];
>> -
>> - tst_parse_opts(ac, av, NULL, NULL);
>> -
>> - setup();
>> -
>> - for (lc = 0; TEST_LOOPING(lc); lc++) {
>> -
>> - /* reset tst_count in case we are looping */
>> - tst_count = 0;
>> -
>> - /*
>> - * Check for ENOSYS.
>> - */
>> - ptr = malloc(10);
>> - func = 100;
>> - retval = modify_ldt(func, ptr, sizeof(ptr));
>> - if (retval < 0) {
>> - if (errno != ENOSYS) {
>> - tst_resm(TFAIL, "modify_ldt() set invalid "
>> - "errno, expected ENOSYS, got: %d",
>> - errno);
>> - } else {
>> - tst_resm(TPASS,
>> - "modify_ldt() set expected errno");
>> - }
>> - } else {
>> - tst_resm(TFAIL, "modify_ldt error: "
>> - "unexpected return value %d", retval);
>> - }
>> -
>> - free(ptr);
>> -
>> - /*
>> - * Check for EINVAL
>> - */
>> - ptr = 0;
>> -
>> - retval = modify_ldt(1, ptr, sizeof(ptr));
>> - if (retval < 0) {
>> - if (errno != EINVAL) {
>> - tst_resm(TFAIL, "modify_ldt() set invalid "
>> - "errno, expected EINVAL, got: %d",
>> - errno);
>> - } else {
>> - tst_resm(TPASS,
>> - "modify_ldt() set expected errno");
>> - }
>> - } else {
>> - tst_resm(TFAIL, "modify_ldt error: "
>> - "unexpected return value %d", retval);
>> - }
>> -
>> - /*
>> - * Create a new LDT segment.
>> - */
>> - if (create_segment(seg, sizeof(seg)) == -1) {
>> - tst_brkm(TBROK, cleanup, "Creation of segment failed");
>> - }
>> -
>> - /*
>> - * Check for EFAULT
>> - */
>> - ptr = sbrk(0);
>> -
>> - retval = modify_ldt(0, ptr + 0xFFF, sizeof(ptr));
>> - if (retval < 0) {
>> - if (errno != EFAULT) {
>> - tst_resm(TFAIL, "modify_ldt() set invalid "
>> - "errno, expected EFAULT, got: %d",
>> - errno);
>> - } else {
>> - tst_resm(TPASS,
>> - "modify_ldt() set expected errno");
>> - }
>> - } else {
>> - tst_resm(TFAIL, "modify_ldt error: "
>> - "unexpected return value %d", retval);
>> - }
>> - }
>> - cleanup();
>> - tst_exit();
>> -}
>> -
>> -/*
>> - * create_segment() -
>> - */
>> -int create_segment(void *seg, size_t size)
>> + unsigned long bytecount;
>> + int exp_errno;
>> +} tst_cases[] = {
> Here tcases[]
>> + /* { 100, &buf, sizeof(buf), ENOSYS }, // 01 block1 */
>> + { 1, (void *)0, 0, EINVAL }, // 01 block2
>> + { 0, &ptr, sizeof(ptr), EFAULT }, // 01 block3
>> +};
>> +
>> +void run(unsigned int i)
>> {
>> - modify_ldt_s entry;
>> + struct tst_case *tc = &tst_cases[i];
>>
>> - 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;
>> -
>> - return modify_ldt(1, &entry, sizeof(entry));
>> + TST_EXP_FAIL(modify_ldt(tc->tfunc, tc->ptr, tc->bytecount),
>> + tc->exp_errno);
>> }
>>
>> void setup(void)
>> {
>> + int seg[4];
>>
>> - tst_sig(FORK, DEF_HANDLER, cleanup);
>> -
>> - TEST_PAUSE;
>> -}
>> -
>> -void cleanup(void)
>> -{
>> -
>> + ptr = sbrk(0) + 0xFFF;
>> + create_segment(seg, sizeof(seg));
>> + tst_cases[1].ptr = ptr;
>> }
>>
>> -#elif HAVE_MODIFY_LDT
>> -int main(void)
>> -{
>> - tst_brkm(TCONF,
>> - NULL,
>> - "modify_ldt is available but not tested on the platform than __i386__");
>> -}
>> +static struct tst_test test = {
>> + .test = run,
>> + .tcnt = ARRAY_SIZE(tst_cases),
>> + .setup = setup,
>> + .bufs =
>> + (struct tst_buffers[]){
>> + { &buf, .size = sizeof(struct user_desc) },
>> + {},
>> + },
>> +};
>>
>> #else
>> -int main(void)
>> -{
>> - tst_resm(TINFO, "modify_ldt01 test only for ix86");
>> - tst_exit();
>> -}
>> -
>> -#endif /* defined(__i386__) */
>> +TST_TEST_TCONF("Test supported only on i386");
>> +#endif /* __i386__ */
>>
> Andrea
More information about the ltp
mailing list