Skip to content

Conversation

cconlon
Copy link
Member

@cconlon cconlon commented Jun 18, 2025

Description

PR #8867 added use of getpid(), and also adjusted the definition of WC_RNG in wolfssl/wolfcrypt/random.h based on HAVE_PID:

#if defined(HAVE_GETPID) && !defined(WOLFSSL_NO_GETPID)
    pid_t pid;
#endif

The HAVE_GETPID define was not being stored to wolfssl/options.h, so applications depending on the sizeof(WC_RNG) would have gotten a different structure size.

This PR modifies configure.ac to define HAVE_GETPID in wolfssl/options.h if getpid() is detected.

Testing

Existing unit tests, and wolfcrypt-jni test cases.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@cconlon cconlon self-assigned this Jun 18, 2025
dgarske
dgarske previously approved these changes Jun 18, 2025
…bundles that do not have older random.c files
@cconlon
Copy link
Member Author

cconlon commented Jun 23, 2025

Retest this please Jenkins

dgarske
dgarske previously approved these changes Jun 23, 2025
@JacobBarthelmeh
Copy link
Contributor

JacobBarthelmeh commented Jun 23, 2025

Retest this please Jenkins. Made a change now for PRB-fips-ready-config to account for HAVE_GETPID macro define when creating the user_settings.h file to test with.

@dgarske
Copy link
Contributor

dgarske commented Jun 23, 2025

We need this PR merged. Worked with Aidan to debug an issue with wolfProvider that turned out to be the same WC_RNG size issue corrupting memory due to struct size mismatch. Multiple engineers have each wasted multi hours because of this change.

@JacobBarthelmeh JacobBarthelmeh removed their assignment Jun 24, 2025
@dgarske
Copy link
Contributor

dgarske commented Jun 24, 2025

Merging before prb master finishes. Thank you Chris and Jacob

@dgarske dgarske merged commit 978a29d into wolfSSL:master Jun 24, 2025
221 of 222 checks passed
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.

4 participants