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

compat/arc4random.c: use memset instead of explicit_bzero #252

Merged
merged 1 commit into from Oct 16, 2023
Merged

Conversation

ffontaine
Copy link
Contributor

Use memset instead of explicit_bzero to avoid the following build failure with uclibc-ng since version 10.0.3 and
837d09e:

/home/fabrice/buildroot/output/host/lib/gcc/arm-buildroot-linux-uclibcgnueabi/12.3.0/../../../../arm-buildroot-linux-uclibcgnueabi/bin/ld: ../compat/arc4random.o: in function `_rs_stir_if_needed':
arc4random.c:(.text+0x8cc): undefined reference to `explicit_bzero'

Use memset instead of explicit_bzero to avoid the following build
failure with uclibc-ng since version 10.0.3 and
837d09e:

/home/fabrice/buildroot/output/host/lib/gcc/arm-buildroot-linux-uclibcgnueabi/12.3.0/../../../../arm-buildroot-linux-uclibcgnueabi/bin/ld: ../compat/arc4random.o: in function `_rs_stir_if_needed':
arc4random.c:(.text+0x8cc): undefined reference to `explicit_bzero'

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
@igalic
Copy link

igalic commented Oct 8, 2023

C23 gives us memset_explicit(), so maybe in 3-10 years we can start using it

@rsmarples rsmarples merged commit 5707073 into NetworkConfiguration:master Oct 16, 2023
5 checks passed
@thithib
Copy link

thithib commented Oct 20, 2023

With this change, chances are the call to memset() will be optimized out by the compiler and the rnd buffer won't be erased. I don't know the security impact of that but I assume there was a good reason to use explicit_bzero() in the first place.

Wouldn't it be safer to leverage feature test macros or the like to keep relying on explicit_bzero() on systems that support it and use alternatives on others?

@rsmarples
Copy link
Member

Happy to look at a PR that adds HAVE_MEMSET_EXPLICT and HAVE_BZERO_EXPLICIT macros set by configure and call the relevant functions accordingly. If anyone is concerned by this in dhcpcd-10.0.4 they can always set CFLAGS=-O0 which should disable any optimisations.

rsmarples added a commit that referenced this pull request Oct 23, 2023
These won't be optimised away by the compiler and our arc4random
compat function should use them *if* available.
If none are then a warning will be emitted to say it's potentially insecure.

Hopefully only uclibc users will see this message.

Fixes #252.
rsmarples added a commit that referenced this pull request Oct 23, 2023
@rsmarples
Copy link
Member

@thithib This is now done and I emitted a warning if we have to fallback to memset.
Only uclibc users (which @ffontaine is using based on his output) will see this warning hopefully.

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.

None yet

4 participants