fix(posix): zero pthread_key_t stack slot before pthread_key_create#13391
fix(posix): zero pthread_key_t stack slot before pthread_key_create#13391gounthar wants to merge 6 commits intooracle:masterfrom
Conversation
StackValue.get(pthread_key_tPointer.class) allocates size_t (8 bytes) because pthread_key_tPointer is @CPointerTo(nameOfCType = "size_t"). pthread_key_create writes only 4 bytes (pthread_key_t is unsigned int on Linux). The subsequent key.read() does an 8-byte load, leaving the upper 32 bits as stack garbage. On amd64 glibc validates the key with a 32-bit cmpl so the upper bits are never examined. On riscv64 glibc uses a 64-bit bltu: a key value like 0x????????00000002 is >= 1024, pthread_setspecific returns EINVAL, and glibc aborts with "pthread_setspecific(key, value): wrong arguments". Zeroing the slot before the call ensures key.read() always returns a clean 32-bit key value regardless of what was on the stack. See: oracle#13386
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
|
Thank you for signing the OCA. |
|
Thanks for the contribution! We'll review this shortly. @wirthi, could you please assign a reviewer? |
|
Appreciate the quick acknowledgment, looking forward to the review. |
Summary
StackValue.get(pthread_key_tPointer.class)allocatessize_tbytes (8 on 64-bit) becausepthread_key_tPointeris@CPointerTo(nameOfCType = "size_t").pthread_key_createwrites only the 4-bytepthread_key_tvalue (unsigned inton Linux). Thenkey.read()does an 8-byte load, so whatever was sitting on the stack in the upper 32 bits gets folded into the returnedThreadLocalKey.On amd64 this is silent: glibc validates the key with a 32-bit comparison and never touches the upper word. On riscv64, glibc uses a 64-bit
bltuagainstPTHREAD_KEYS_MAX(1024). A key like0xdeadbeef00000002fails that check,pthread_setspecificreturnsEINVAL, and glibc aborts duringIsolateThreadCache.clear()at shutdown:Zeroing the slot before the call fixes this on my setup. I'm not sure it's the right approach though. Zeroing at allocation time might be cleaner, or maybe
pthread_key_tPointershould be sized to reflect the actual 4-byte type rather thansize_t. Happy to take direction from anyone who knows this area better.Related Issues
Fixes #13386
Testing
Tested on a single Banana Pi F3 (SpacemiT K1, rv64gc, Debian). With the fix, a HelloWorld native binary runs and exits cleanly. Without it, it prints output and then aborts with the error above.
I haven't tested other riscv64 boards or glibc versions, so there may be cases I haven't hit. On amd64 the change should be a no-op since the 32-bit glibc comparison never reads the upper bytes, but I haven't run a full test suite to confirm nothing regresses there. A proper regression test would need a riscv64 CI runner, which I don't have access to.
Documentation
Nothing to update. Runtime crash fix, no API change.
Contributor Checklist