fix(vmm/sandbox): avoid intermediate overflow in checked_compute_delta - #249
fix(vmm/sandbox): avoid intermediate overflow in checked_compute_delta#249thanhtoantnt wants to merge 1 commit into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the checked_compute_delta function in vmm/sandbox/src/client.rs to prevent intermediate integer overflows by widening i64 values to i128 during calculation. It also adds several unit tests to verify correct behavior with extreme values like i64::MAX and i64::MIN. The review feedback suggests simplifying the code by using a direct cast to i64 instead of try_from, as the result is mathematically guaranteed to fit within the i64 range, and removing redundant parentheses for better readability.
There was a problem hiding this comment.
Pull request overview
This PR fixes an integer overflow bug in checked_compute_delta used during host/guest clock synchronization by widening intermediate arithmetic so large timestamp deltas no longer fail when the final averaged result still fits in i64 (Fixes #244).
Changes:
- Compute
delta_client + delta_serverini128to prevent intermediatei64overflow before dividing by 2. - Keep subtraction overflow protection via
checked_sub, while making the final conversion back toi64explicit viatry_from. - Add regression tests covering
i64::MAX,i64::MIN, and a subtraction-overflow error case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c453bcf to
b86a64a
Compare
|
@novahe Could you review this fix? |
thanks! nit: please squash commits |
The intermediate sum delta_client + delta_server could overflow i64 even when the final result (sum / 2) fits in i64. For example: checked_compute_delta(i64::MAX, 0, 0, i64::MAX) should return Ok(i64::MAX) but previously returned Err due to overflow. Fix: widen both operands to i128 before adding. The sum of two i64 values always fits in i128, and after dividing by 2 the result is guaranteed to fit back in i64, so a direct as i64 cast is used. Also add three regression tests covering the i64::MAX, i64::MIN, and subtraction-overflow cases. Fixes kuasar-io#244 Signed-off-by: thanhtoantnt <thanhtoantnt@gmail.com>
Done. |
|
@novahe could you review this PR? |
The intermediate sum delta_client + delta_server could overflow i64 even when the final result (sum / 2) fits in i64. For example:
checked_compute_delta(i64::MAX, 0, 0, i64::MAX)
should return Ok(i64::MAX) but previously returned Err due to overflow.
Fix: widen both operands to i128 before adding. The sum of two i64 values always fits in i128, and after dividing by 2 the result always fits back in i64, so the try_from cast is infallible in practice.
Also add three regression tests covering the i64::MAX, i64::MIN, and subtraction-overflow cases.
Fixes #244