-
Notifications
You must be signed in to change notification settings - Fork 39
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: fixes hanging firstboot with kernel 6.1y #71
Conversation
Is there no |
I have to check that, but what we basicly do is the same procedure as octopi and addings stuff due guysoft/custompios into the existing Raspberry Pi OS Image in a chroot env. So, if it is missing I think it should already a part of it. |
I checked it and neither in our image nor raspi os lite. |
Because what I'm just not sure whether the daemon runs already at this early boot stage. It has default dependencies, but no additional ordering: root@micha:~# systemctl cat rngd
# /lib/systemd/system/rngd.service
[Unit]
Description=Start entropy gathering daemon (rngd)
Documentation=man:rngd(8)
[Service]
ExecStart=/usr/sbin/rngd -f
[Install]
WantedBy=multi-user.target Reading from the hardware random generator device |
I really understand what you want to improve here and I appriciate that, but I simply modified to get it working what exists already. If that is unwanted, then Raspberry Pi OS has to overthink the whole concept at some point. We dont build the base Image on our self, we use "of the shelf " what is available via download from the Pi Imager. Dont want argue to strong here, yes possible solution for a running OS, but not on firstboot. |
Ah okay, then indeed it is not running and using |
Thats exactly what I thought also, a partuuid isnt that crutial as a hash for ssh. So, yes urandom should be sufficiant enough. I used 256 bytes only to "grant" some what of real randomness. |
To avoid conflicts with non rpi images, moved to seperate modules Can be removed if solved due merge of RPi-Distro/raspberrypi-sys-mods#71 Signed-off-by: Stephan Wendel <me@stephanwe.de>
@@ -63,7 +63,11 @@ get_variables () { | |||
fix_partuuid() { | |||
mount -o remount,rw "$ROOT_PART_DEV" | |||
mount -o remount,rw "$BOOT_PART_DEV" | |||
DISKID="$(tr -dc 'a-f0-9' < /dev/hwrng | dd bs=1 count=8 2>/dev/null)" | |||
MAJOR="$(uname -r | cut -f1 -d.)" | |||
if [[ "$MAJOR" -eq "6" ]] && [[ -c /dev/hwrng ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does the major version matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably because the slowdown appeared (and is now fixed) in 6.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But this is saying if running 6.0 and newer, inject potentially slow hwrng into urandom. If the idea was to avoid the slowdown, then shouldn't that be negated? In either case, especially with the kernel fix, I can't think of a reason to have different behaviour in these two cases. So I'm tempted to just skip hwrng and only use urandom for the ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My Idea behind that was to keep the original as much as possible, but I also dont wnated to get completly rid of some hardware generated symbols/chars. I never tested it with time measuring in mind. I only want to get rid of that 'hanging' behavior.
Therefor I used the major version because 5.15y wasnt affected by that.
FYI the kernel change that caused this catastrophic slowdown has already been reverted and is due to be replaced with a proper patch. This is likely to be a one-off problem, so I question the need for the change (and indeed the possibility of creating a system that produces cryptographically good random numbers quicker than the HWRNG block, or even at an acceptably slower rate). |
In this particular case I don't think the quality of the random numbers matters. AIUI, uranodom is a lot faster and will have more than enough entropy to avoid generating sd card with the same ID. So I'm thinking we should just use urandom directly without injecting anything extra from hwrng. That seems to save 2 seconds on a pi 4. Or would you still advise we keep it as is? |
I think we all agree with this and the feeding from hwrng is only done to preserve the "true" randomness at least partly, in case it was done for some reason before.
AFAIK, the chance that two UUIDs will be identical is exactly the same with hwrng, random and urandom. The only difference is that the latter uses sources which are known and hence the result is theoretically predictable and hence not suitable for cryptographic keys and such. So yes, just replace |
Isn't much of the time wasted in pre-reading data which is never used due to the way the pipeline is constructed? What's wrong with:
Feel free to substitute /dev/urandom if that is still preferred. |
|
The |
Yeah, your version is much better. In this case there's no difference between the speed of urandom and hwrng, so if hwrng is a higher entropy source, then I see no reason to switch to urandom. I didn't like it either throwing data away either. I blame whoever I stole it from on stackoverflow. |
Oh wow, I should have tested the whole pipe. Nice tool
With only 4 characters it doesn't make a difference, but also using urandom doesn't make anything worse. As said, it is not worse in terms of producing results that differ from each other, it is only worse in terms of making it possible for attackers to predict or reconstruct what has been produced. But the UUID is not a secret and can be obtain by any unprivileged user anyway. |
Logging the number of characters read from hwrng, here's the old version:
a total of 128KB, and the new one:
a total of 4. |
Reading the entire discussion, sounds to me ( and I am no native english speaker ), that changes suggested by @pelwell , using If yes I would prepare a commit to change to that version. Regards Kwad |
Yes, that would be much appreciated. Thank you! |
As discussed in RPi-Distro#71 changed behavior to mentioned code by pelwel. See RPi-Distro#71 (comment) for details. Signed-off-by: Stephan Wendel <me@stephanwe.de>
@XECDesign Regards Kwad |
As discussed in RPi-Distro#71 changed behavior to mentioned code by pelwel. See RPi-Distro#71 (comment) for details. Signed-off-by: Stephan Wendel <me@stephanwe.de>
6e82fa5
to
015da43
Compare
Thanks! |
This reverts changes made in #214 as described in #213. Fix is already merged in RPi-Distro/raspberrypi-sys-mods#71 Raspberry hasn't released updated image yet, but should work as we upgrade during build ci. Signed-off-by: Stephan Wendel <me@stephanwe.de>
This reverts changes made in #214 as described in #213. Fix is already merged in RPi-Distro/raspberrypi-sys-mods#71 Raspberry hasn't released updated image yet, but should work as we upgrade during build ci. Signed-off-by: Stephan Wendel <me@stephanwe.de>
This fixes issue with preinstalled kernel 6.1y,
which leads to an hanging first boot in function
fix_partuuid
on Raspberry Pi Zero 2 W and Pi 3 series.Instead of waiting for
/dev/hwrng
to produce enough symbols, force it to write to/dev/urandom
and read from there.This should fix #70