[LTP] [PATCH] misc: rewrite crash02 test

[Martin Doucha]

On 23. 06. 25 10:15, Andrea Cervesato wrote:
>>>   -    if (l_opt)
>>> -        sysno_max = atoi(l_copt);
>>> +    num = (buff[0] << 24) | (buff[1] << 16) | (buff[2] << 8) | buff[3];
>>> +    if (num < 0)
>>> +        num *= -1;
>>> +    num = (num % MAX_SYSCALLS) - 1;
>>
>> Why do you subtract on the last line above? 
> Some syscalls need to set their argument to -1 sometimes.

OK, we should test the whole value range, though. Other negative values 
might trigger corner cases we're still interested in.

>> Also, this would be much simpler and the truncation to MAX_SYSCALLS 
>> should be done by caller when needed.
>>
>> static inline long rand_long(void)
>> {
>>     long ret;
>>
>>     if (getrandom(&ret, sizeof(ret), 0) < 0)
>>         tst_brk(...);
>>
>>     return ret;
>> }
>>
>> Although it's be great to support setting random seed like in the 
>> original test so that crashes can be reproduced.
> 
> I don't think I understand this sentence. This code is taking into 
> account the specific numeric arch size, by casting a 64bit number to 
> "long" type. long has variable bytes and it's eventually cut during the 
> cast.
> 
> The crash can be reproduced because we are TDEBUG the full syscall 
> address and arguments which are given to it.

Some crashes may need multiple syscalls to set up. Running
crash02 -s $rnd_seed
allows for easier reproduction than copy-pasting the entire call log 
into a new test.

>>> +    do {
>>> +        invalid = 0;
>>> +        sysno = rand_number() % MAX_SYSCALLS;
>>> +
>>> +        for (size_t i = 0; i < ARRAY_SIZE(blacklist); i++) {
>>> +            if (blacklist[i] == sysno) {
>>> +                invalid = 1;
>>> +                break;
>>> +            }
>>>           }
>>
>> The original approach with in_blacklist() helper function was cleaner. 
>> Also note that both your rand_number() and my rand_long() can return 
>> negative values.
> They can return -1 only. All the negative values below this value are 
> converted to positive numbers.

Returning -1 might be enough to cause trouble here. While you did add 
__LTP__NR_INVALID_SYSCALL to the blacklist, if the constant changes, 
you'll end up calling syscall(-1, ...) on some archs and 
syscall(MAX_SYSCALLS-1, ...) on others. The C standard doesn't define 
which of these values is the correct result of modulo operation with a 
negative left operand.

-- 
Martin Doucha   mdoucha@suse.cz
SW Quality Engineer
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic