kernel: sched: fix possible integer overflow in z_tick_sleep()

[sudarsan-22]

Fix Coverity CID 529867 (CWE-190): z_tick_sleep() may return a large tick count due to wraparound during unsigned tick subtraction.

This patch replaces unsigned subtraction with signed arithmetic to safely handle tick count wraparound and avoid returning incorrect values after timeout abortion.

Fixes: #92601

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[andyross]

It looks to me like this is going to generate the same code and the same operations: it does a uint32_t subtraction and manually checks for overflow by casting the results to int32_t and looking at the sign.

What's the argument for why Coverity won't just flag it again? If the argument is "Coverity is too dumb to notice this new code", then IMHO the proper solution is to filter the test and not just rearrange the code.

If you need to test for overflow in unsigned math in a robust way, the solution is generally something like:

if (a > b) return a - b else return 0;

[sudarsan-22]

It looks to me like this is going to generate the same code and the same operations: it does a uint32_t subtraction and manually checks for overflow by casting the results to int32_t and looking at the sign.

What's the argument for why Coverity won't just flag it again? If the argument is "Coverity is too dumb to notice this new code", then IMHO the proper solution is to filter the test and not just rearrange the code.

If you need to test for overflow in unsigned math in a robust way, the solution is generally something like:

if (a > b) return a - b else return 0;

Thanks Andy for the review!

You're right — the updated code still performs unsigned subtraction internally, and it's possible that Coverity may flag this again despite the cast.

The explicit check you suggested using:

if (expected_wakeup_ticks > now) {
return expected_wakeup_ticks - now;
} else {
return 0;
}
which is ok ?

[andyross]

If you're able to run coverity locally, then the right answer is "whatever works to silence the error". If not, sticking to defined behavior is best, and what you wrote in your comment looks correct to me. Sleeping isn't in general a performance path so we don't need to care too much about generated code quality.