[LTP] [PATCH v8 1/3] syscalls/modify_ldt: Add lapi/ldt.h

[Ricardo B. Marlière]

Hi !

On Tue Apr 29, 2025 at 4:40 AM -03, Andrea Cervesato wrote:
> Hi Ricardo,
>
> On 4/8/25 13:26, Ricardo B. Marlière wrote:
>> From: Ricardo B. Marlière <rbm@suse.com>
>>
>> Add a wrapper to modify_ldt and a fallback definition to struct user_desc,
>> which are needed in a few tests and should be reused.
>>
>> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
>> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>
>> Signed-off-by: Ricardo B. Marlière <rbm@suse.com>
>> ---
>>   include/lapi/ldt.h                      | 58 +++++++++++++++++++++++++++++++++
>>   testcases/cve/cve-2015-3290.c           | 26 ++-------------
>>   testcases/cve/cve-2017-17053.c          |  6 ++--
>>   testcases/kernel/syscalls/fork/fork05.c |  5 ++-
>>   4 files changed, 64 insertions(+), 31 deletions(-)
>>
>> diff --git a/include/lapi/ldt.h b/include/lapi/ldt.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..cfb98883633b4048babb93c83f09c6b413c004f6
>> --- /dev/null
>> +++ b/include/lapi/ldt.h
>> @@ -0,0 +1,58 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Copyright (c) 2025 SUSE LLC Ricardo B. Marlière <rbm@suse.com>
>> + */
>> +
>> +#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 */
>> +
>> +static inline int modify_ldt(int func, const struct user_desc *ptr,
>> +			     unsigned long bytecount)
>> +{
>> +	return tst_syscall(__NR_modify_ldt, func, ptr, bytecount);
>> +}
>> +
>> +static inline int safe_modify_ldt(const char *file, const int lineno, int func,
>> +				  const struct user_desc *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);
>> +	else if (rval)
>> +		tst_brk_(file, lineno, TBROK | TERRNO,
>> +			 "modify_ltd(%d, %p, %lu) invalid retval %i", func, ptr,
>> +			 bytecount, rval);
>
> The LTP/Kernel coding convention is that a statement using more than one 
> line needs to be surrounded by braces: 
> https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation
> The rest looks ok.

I thought braces were not necessary for single statements blocks, it
does not mention multi-line single statements. Checkpatch.pl does not
say anything:

$ b4 prep --check
Checking patches using:
  /mnt/ext/src/linux/ltp/conversions/modify_ldt/scripts/checkpatch.pl -q --terse --no-summary --mailback
 --showfile --no-tree --ignore CONST_STRUCT,VOLATILE,SPLIT_STRING,FILE_PATH_CHANGES
---
● 23e78431927d: syscalls/modify_ldt: Add lapi/ldt.h
  ● checkpatch.pl: include/lapi/ldt.h:1: WARNING: Improper SPDX comment style for 'include/lapi/ldt.h', 
please use '/*' instead
  ● checkpatch.pl: include/lapi/ldt.h:1: WARNING: Missing or malformed SPDX-License-Identifier tag in li
ne 1
● 26016810e17f: syscalls/modify_ldt02: Refactor into new API
  ● checkpatch.pl: testcases/kernel/syscalls/modify_ldt/common.h:1: WARNING: Improper SPDX comment style
 for 'testcases/kernel/syscalls/modify_ldt/common.h', please use '/*' instead
  ● checkpatch.pl: testcases/kernel/syscalls/modify_ldt/common.h:1: WARNING: Missing or malformed SPDX-L
icense-Identifier tag in line 1
● 9d33db29e43b: syscalls/modify_ldt01: Refactor into new API
---
Success: 1, Warning: 4, Error: 0

>
> Reviewed-by: Andrea Cervesato <andrea.cervesato@suse.com>

Thanks for reviewing,
-	Ricardo.


>
> - Andrea