On 6/23/25 1:40 PM, Martin Doucha wrote:
> 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.
What is not convincing me about this approach is that we might stuck
into negative values by generating random numbers.
The test should run for many hours or even days. And that's something we
already have for syzkaller (not used by SUSE but by many other companies).
>
>>> 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.
Ok now I understood. Yes, I can add that.
>
>>>> + 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.
>
I can set to __LTP_NR_INVALID_SYSCALL all the times I obtain a negative
value. The loop will restart, the syscall will be valid. But only for
syscalls. For arguments we need to decided if it's the case to run with
every possible negative value (discussione above).
- Andrea