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

Broken RDRAND causes infinite looping #588

Closed
Xyene opened this issue May 2, 2020 · 2 comments · Fixed by #589
Closed

Broken RDRAND causes infinite looping #588

Xyene opened this issue May 2, 2020 · 2 comments · Fixed by #589

Comments

@Xyene
Copy link
Contributor

Xyene commented May 2, 2020

Some CPUs, like the Ryzen 3600X, report having RDRAND in cpuid, but actually executing RDRAND returns an endless stream of 0xFFFFFFFF. This causes json-c to lock up retrying RDRAND, but never trying another source of entropy.

json-c/linkhash.c

Lines 467 to 469 in 31f1ab2

/* we can't use -1 as it is the unitialized sentinel */
while ((seed = json_c_get_random_seed()) == -1) {}
#if SIZEOF_INT == 8 && defined __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8

A number of Arch Linux users reported issues with this in swaywm/sway#5290, where json-c was recently upgraded from 0.13 to 0.14. There, this problem manifested as the window manager entirely locking up during JSON IPC.

The kernel supports a nordrand flag, but that has no effect on the availability of RDRAND signaled by cpuid. The workaround here is to update BIOS to a version that fixes this issue, but considering the number of reports in that project alone, it might be worthwhile to include fallback logic in json-c itself, and use a different random source if RDRAND fails too many times. I'm happy to submit such a patch, if desired.

Xyene added a commit to Xyene/json-c that referenced this issue May 2, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
Xyene added a commit to Xyene/json-c that referenced this issue May 2, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
@hawicz
Copy link
Member

hawicz commented May 3, 2020

oof, yeah, a patch to disable the use of RDRAND would be quite welcome. Perhaps check, in random_seed.c, for Ryzen CPUs, and if so, check whether the first dozen or so calls to RDRAND all return 0xFFFFFFFF, and don't use it in that case?

@hawicz
Copy link
Member

hawicz commented May 3, 2020

Oh, just noticed you have a PR #589 open. I'm closing this ticket in favor of that one.

@hawicz hawicz closed this as completed May 3, 2020
Xyene added a commit to Xyene/json-c that referenced this issue May 3, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
Xyene added a commit to Xyene/json-c that referenced this issue May 3, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
Xyene added a commit to Xyene/json-c that referenced this issue May 3, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
besser82 pushed a commit to besser82/json-c that referenced this issue May 10, 2020
Some CPUs advertise RDRAND in CPUID, but return 0xFFFFFFFF
unconditionally. To avoid locking up later, test RDRAND during
initialization, and if it returns 0xFFFFFFFF, mark it as nonexistent.

Fixes json-c#588.
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 a pull request may close this issue.

2 participants