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

Provide a way to enable getrandom on Linux even when build system runs an older kernel #72177

Closed
ncoghlan opened this issue Sep 7, 2016 · 9 comments
Labels
type-feature A feature request or enhancement

Comments

@ncoghlan
Copy link
Contributor

ncoghlan commented Sep 7, 2016

BPO 27990
Nosy @ncoghlan, @vstinner, @encukou, @vadmium

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-09-21.09:02:30.161>
created_at = <Date 2016-09-07.02:46:39.022>
labels = ['type-feature']
title = 'Provide a way to enable getrandom on Linux even when build system runs an older kernel'
updated_at = <Date 2016-09-21.12:55:09.273>
user = 'https://github.com/ncoghlan'

bugs.python.org fields:

activity = <Date 2016-09-21.12:55:09.273>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2016-09-21.09:02:30.161>
closer = 'ncoghlan'
components = []
creation = <Date 2016-09-07.02:46:39.022>
creator = 'ncoghlan'
dependencies = []
files = []
hgrepos = []
issue_num = 27990
keywords = []
message_count = 9.0
messages = ['274715', '274727', '274758', '277072', '277113', '277120', '277122', '277123', '277131']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'vstinner', 'petr.viktorin', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27990'
versions = ['Python 3.6']

@ncoghlan
Copy link
Contributor Author

ncoghlan commented Sep 7, 2016

The configure script determines the setting for HAVE_GETRANDOM_SYSCALL at build time, which means the dynamic check for getrandom() support in the Linux kernel gets disabled when building against an older kernel.

This impacts the implicit use of getrandom() in os.urandom(): https://mail.python.org/pipermail/security-sig/2016-June/000060.html

And also the new os.getrandom() API added in bpo-27778: http://bugs.python.org/issue27778#msg274698

It's desirable to have a way of forcing the inclusion of the dynamic runtime check, even if the currently running kernel doesn't provide the syscall itself.

@ncoghlan ncoghlan added the type-feature A feature request or enhancement label Sep 7, 2016
@vstinner
Copy link
Member

vstinner commented Sep 7, 2016

Support for the getrandom() syscall requires syscall(), SYS_getrandom constant, GRND_NONBLOCK and GRND_RANDOM constants, linux/random.h header, etc.

I don't understand your request. configure doesn't check getrandom() syscall result:

    /* ignore the result, Python checks for ENOSYS and EAGAIN at runtime */
    (void)syscall(SYS_getrandom, buffer, buflen, flags);

I don't see how you can support getrandom() if the host doesn't have all constants to compile the C code. Do you have to hardcode constants?

@ncoghlan
Copy link
Contributor Author

ncoghlan commented Sep 7, 2016

It may be that the right fix is to build with more recent Linux header files, even when running the build on an older kernel, in which case the request would be to have a way to indicate that missing those particular headers should fail the build, rather than implicitly omitting os.getrandom().

Worst case, yeah, we'll have to patch Fedora's system Python to hardcode the necessary constants in order to cope with the way Koji's buildroots are currently set up. I hope it doesn't come to that, though.

I'll leave the specifics to Petr, but it would definitely be preferable to have a common way of solving this recommended by upstream rather than everyone affected needing to figure out how to solve the problem on their own.

@vstinner
Copy link
Member

I'm not excited by the idea of using hardcoded constants for getrandom(). There is a risk of using wrong constants depending on the architecture or the Linux kernel version.

The code is already very low-level: it calls directly the syscall using syscall() function. getrandom() has no nice glibc clean API yet:
https://sourceware.org/bugzilla/show_bug.cgi?id=17252

I suggest to close the issue as WONTFIX. It's ok to use a file descriptor and read /dev/urandom. getrandom() is nice to have, but it's not really a killer feature.

For Fedora: sure, you can use vendor patches to workaround your technical issue, builders using an old kernel :-) But it would be simpler to upgrade the builder, no? :-)

@ncoghlan
Copy link
Contributor Author

I'm happy to close this as not even being a bug - the Fedora Python maintainers found that it was only specifically 3.5.1 that wasn't finding the necessary header files in the Fedora build root (and hence not even trying the syscall), while 3.5.0 and 3.5.2 were both fine.

@vstinner
Copy link
Member

Oh it's strange that only 3.5.1 has the issue. Happy to read that the
problem is already solved!

@encukou
Copy link
Member

encukou commented Sep 21, 2016

Indeed, I'd close WONTFIX.

IMO, applications that:

  • run at early boot, and
  • get built with an older kernel than they run on
    fall squarely into the enterprise distro turf, and CPython code shouldn't include hacks needed to make this work. That's not to say I'm opposed to collaboration if anyone else needs to solve this problem – quite the opposite. But I think a vendor-specific patch is the right layer here.

As currently documented, the getrandom call is not used on older kernels.

(Btw, no, it's not easy for *me* to help upgrading Fedora builders. But I can easily maintain a vendor patch for Python, if it comes to that.)

@encukou
Copy link
Member

encukou commented Sep 21, 2016

(Please ignore that comment – I was on vacation and, when clearing my backlog, got to this issue before the Fedora discussions.)

@vstinner
Copy link
Member

Ok, I changed the status to WONTFIX.

@vstinner vstinner removed the invalid label Sep 21, 2016
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

3 participants