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 build with musl and older Linux kernel #82

Closed
wants to merge 1 commit into from

Conversation

baruchsiach
Copy link

The musl libc carries its own copy of Linux system calls. When building
with Linux headers older than v3.17, musl provides SYS_getrandom
definition, but not GRND_NONBLOCK. This causes build failure for
libressl and openntpd:

getentropy_linux.c: In function 'getentropy_getrandom':
getentropy_linux.c:205:42: error: 'GRND_NONBLOCK' undeclared (first use in this function)
ret = syscall(SYS_getrandom, buf, len, GRND_NONBLOCK);
^~~~~~~~~~~~~

Define GRND_NONBLOCK locally when its definition is missing to fix the
build. There should be no run-time effect. Older kernels return ENOSYS
for unsupported syscall().

The musl libc carries its own copy of Linux system calls. When building
with Linux headers older than v3.17, musl provides SYS_getrandom
definition, but not GRND_NONBLOCK. This causes build failure for
libressl and openntpd:

getentropy_linux.c: In function 'getentropy_getrandom':
getentropy_linux.c:205:42: error: 'GRND_NONBLOCK' undeclared (first use in this function)
   ret = syscall(SYS_getrandom, buf, len, GRND_NONBLOCK);
                                          ^~~~~~~~~~~~~

Define GRND_NONBLOCK locally when its definition is missing to fix the
build. There should be no run-time effect. Older kernels return ENOSYS
for unsupported syscall().
@sanmai-NL
Copy link

@4a6f656c Who will handle this PR?

@busterb
Copy link

busterb commented Feb 28, 2018

Just so I understand, what prevents you from just using newer headers? Musl AFAIK does not actually use Kernel headers itself for enabling features at all, unlike Glibc.

@baruchsiach
Copy link
Author

@busterb When the target system uses older kernel I prefer to use kernel headers of the same version to minimize the risk of breakage.

But regardless, the entire function is under the #ifdef SYS_getrandom condition. So the code intends to keep compatibility with older headers. It is just that this build time condition is not enough for the musl libc case.

@busterb
Copy link

busterb commented Feb 28, 2018

OK, I can see that. It's odd if Musl is defining syscall definitions itself that are accessible from userspace software, shadowing the kernel syscall definitions.

@4a6f656c
Copy link

This seems like an issue with the musl headers - defining SYS_getrandom but not the values for its flags is a recipe for these kinds of problems. Furthermore, the proposed fix is somewhat dangerous since there is no guarantee that the kernel API matches the values that are being defined (or if that functionality is supported), which could lead to a blocking call (or some other unintended behaviour).

If we want to support this (somewhat broken) use case, then it probably should be with:

#if defined(SYS_getrandom) && defined(GRND_NONBLOCK)

@busterb
Copy link

busterb commented Feb 28, 2018

That works for me @4a6f656c since we do need both. If you don't have the right kernel for this, you'll get an ENOSYS anyway, so the path will fall through in a suboptimal way. Previously, we wouldn't compile the fall back code path if the syscall was defined.

@baruchsiach
Copy link
Author

Should I post an updated pull request along the lines of @4a6f656c suggestion?

@busterb
Copy link

busterb commented Feb 28, 2018

No need, this will go into upstream CVS. This repo is just a mirror, so git pull requests don't apply anyway.

busterb pushed a commit that referenced this pull request Mar 13, 2018
…getrandom(2)

Based on discussion here #82
Suggested fix from jsing@
@busterb
Copy link

busterb commented Mar 13, 2018

Updated upstream, thanks all!

@busterb busterb closed this Mar 13, 2018
bob-beck pushed a commit to openbsd/src that referenced this pull request Mar 14, 2018
…getrandom(2)

Based on discussion here libressl/openbsd#82
Suggested fix from jsing@
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