Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validation: fix delta calculation in kernel mode #139

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thillux
Copy link
Contributor

@thillux thillux commented Jan 30, 2025

No description provided.

@joshuaehill
Copy link
Contributor

joshuaehill commented Jan 30, 2025

In C, arithmetic using unsigned integers is defined as being done mod the maximum value +1 (here, mod 2^64). As such, you don't need to bother with checking for rollover, it just works. In particular, because next and prev are both of type uint64_t, the expression next - prev is equivalent to UINT64_MAX - prev + 1 + next for all values of next and prev.

The prior code also worked, but that seems to have been by accident.

For a modern reference, see Section 6.2.5 paragraph 9 of a recent C spec. This document is the last public ballot document eventually became C18, but this behavior is not new; essentially the same text is in the same section of the C99 standard, very similar text is also present in the the first ANSI C document (ANSI X3.159-1989), and K&R's 1978 book includes the quote "unsigned numbers obey the laws of arithmetic modulo 2^n, where n is the number of bits in an int".

@joshuaehill
Copy link
Contributor

joshuaehill commented Jan 30, 2025

Also, note that there is very similar code in tests/raw-entropy/validation-restart-kernel/extractlsb.c, and this should be treated similarly.

@joshuaehill
Copy link
Contributor

For nominally broader context (but really I didn't see anything new there), see Issue #18.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants