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

Teo Couprie Diaz
2023-02-15T15:35:15Z

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 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.

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");