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

Fix integer overflow in memory calculations on 32bit systems #2770

Merged
merged 1 commit into from Nov 27, 2023

Conversation

pmatilai
Copy link
Member

"long int" on at least x86 Linux is exactly the same as a plain "int", so calculations can easily overflow. 32bit systems are not expected to have hundreds of gigabytes of memory but a 32bit process on a x86_64 can run into all sorts of funny things, such having 500GB system memory. At which point the type capable of addressing all of process memory is absolutely useless for calculating the total memory.

Use uint64_t in the memory calculations to make the size explicit. Of course one day we may cross that border too, but I hope to be retired by then.

Fixes https://issues.redhat.com/browse/RHEL-16557

@pmatilai pmatilai added the bug label Nov 16, 2023
"long int" on at least x86 Linux is exactly the same as a plain "int",
so calculations can easily overflow. 32bit systems are not expected to
have hundreds of gigabytes of memory but a 32bit process on a x86_64 can
run into all sorts of funny things, such having 500GB system memory. At
which point the type capable of addressing all of process memory is
absolutely useless for calculating the total memory.

Use uint64_t in the memory calculations to make the size explicit. Of course
one day we may cross that border too, but I hope to be retired by then.

Fixes https://issues.redhat.com/browse/RHEL-16557
@pmatilai
Copy link
Member Author

pmatilai commented Nov 27, 2023

Actually this "fix" was just as broken as it was before, because:
mem = (uint64_t)(pages * pagesize) is just as bad as not casting it at all because it casts the result of the multiplication, whereas we want to force (one of) the members to 64bit 🤦

I love you C, but there are times...

@pmatilai
Copy link
Member Author

Anyway, actually tested now, merging.

@pmatilai pmatilai merged commit 1cd9f90 into rpm-software-management:master Nov 27, 2023
1 check passed
@pmatilai pmatilai deleted the lalalong branch November 27, 2023 13:56
@dmnks dmnks added the packaging Package building, SPEC files, etc. label Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug packaging Package building, SPEC files, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants