[LTP] [PATCH] cve-2015-3290: Fix tst_syscall() return value

[Richard Palethorpe]

Teo Couprie Diaz <teo.coupriediaz@arm.com> writes:

> Hi all,
>
> On 15/02/2023 15:20, Petr Vorel wrote:
>> Hi all,
>>
>>> The modify_ldt() syscall returns 32-bit signed integer value. Recent changes
>>> in tst_syscall() caused the value to be interpreted as unsigned on older
>>> kernels/glibc, which breaks the cve-2015-3290 test. Add explicit type cast
>>> to fix it.
>> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>>
>> This is caused by e5d2a05a9 ("regen.sh: Use intptr_t for tst_syscall return")
>> which changed returning tst_ret from int to intptr_t (which is also int for 32
>> bit archs, but long for 64 bit archs). This commit is also needed, thus I don't
>> suggest to revert it, but I wonder how many other tests it broke.

I think modify_ldt is special. You could search for other syscalls which
only return 32bit to make sure.

>
> I sent the tst_syscall return patch. I did some testing with what systems I
> had available when working on it and, at the time, didn't see any
> regressions,
> including for cve-2015-3290.
>
> However this was with fairly recent kernels and libcs, and according
> to Martin
> the test failed for them on an older combination.
>
> I shared the test suite[0] I tested the patch with, containing all the
> uses I found.
> It might be worthwhile to run it on a system where we now know one of
> the tests
> was affected ? Hopefully, none more, but having a result would be
> better.

I think the testing you did was more than adequate.

>
> Best regards,
> Téo
>
> [0]: https://lists.linux.it/pipermail/ltp/2022-November/031640.html
>
>>
>> Kind regards,
>> Petr
>>
>>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>>> ---
>>>   testcases/cve/cve-2015-3290.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>> diff --git a/testcases/cve/cve-2015-3290.c b/testcases/cve/cve-2015-3290.c
>>> index f61d2809b..a2a8fcedd 100644
>>> --- a/testcases/cve/cve-2015-3290.c
>>> +++ b/testcases/cve/cve-2015-3290.c
>>> @@ -195,7 +195,8 @@ static void set_ldt(void)
>>>   		.useable	 = 0
>>>   	};
>>> -	TEST(tst_syscall(__NR_modify_ldt, 1, &data_desc, sizeof(data_desc)));
>>> +	TEST((int)tst_syscall(__NR_modify_ldt, 1, &data_desc,
>>> +		sizeof(data_desc)));
>>>   	if (TST_RET == -EINVAL) {
>>>   		tst_brk(TCONF | TRERRNO,
>>>   			"modify_ldt: 16-bit data segments are probably disabled");


-- 
Thank you,
Richard.