Page MenuHomeFreeBSD

Fix secure netboot with UEFI
Needs ReviewPublic

Authored by sjg on Mon, Jun 30, 6:01 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jul 7, 10:14 AM
Unknown Object (File)
Sun, Jul 6, 10:40 AM
Unknown Object (File)
Sat, Jul 5, 10:36 PM
Subscribers

Details

Reviewers
stevek
imp
kevans
andrew
manu
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Summary

While netbooting with loader.efi on at least one arm64 platform
which uses u-boot emulating UEFI, the kernel gets corrupted, we
suspected the u-boot ethernet driver is still running.

The change to efinet_dev_init() allows net_cleanup() to be called via
dev_cleanup(), to ensure the u-boot ethernet driver is shut donw.
This however resulted in a loader crash.
Calling dev_cleanup() before bi_load() avoids that crash, and boot
completes successfully.

The rest is changes that should have been upstreamed ages ago.

When doing file verification, tftp needs to be able to handle multiple
open files concurrently.
We also need tftp_stat() to provide useful values for st_dev and st_ino.

Allow an architecture to define NETPROTO_DEFAULT.
The default is NET_NFS for backwards compatability.

In net_parse_rootpath() fix parsing of
<scheme>://<ip>[:<port]/<path>
and ensure we return INADDR_NONE unless we successfully
parsed an addr, so we don't end up clobbering rootip obtained
from bootp().

In install() if proto has not been determined, but
netproto is NET_TFTP set proto to tftp_fsops.

Extra debug tracing to help work out all the above.
libsa/bootp.c could not use BOOTP_DEBUG without common/dev_net.c
having NETIF_DEBUG.
Put a common DEBUG_PRINTF impl in stand.h so we can over time have
more consistent approach to debugging bits of the loader.

Sponsored by: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 65140
Build 62023: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Mon, Jun 30, 6:01 PM
sjg requested review of this revision.Mon, Jun 30, 6:01 PM

Make the same change for arm, i386 and riscv

This mixes a lot of different types of changes all in one big ball that's hard to review.
Any chance you could break them down to approximately what the paragraphs of the description say? At the very least, the debug stuff should be a separate commit, but really there's about 10 commits hiding in this one review by my quick count. It makes bisecting quite a bit simpler to do separate commits.

stand/libsa/tftp.c
671

This seems gratuitous

In D51094#1168160, @imp wrote:

This mixes a lot of different types of changes all in one big ball that's hard to review.
Any chance you could break them down to approximately what the paragraphs of the description say? At the very least, the debug stuff should be a separate commit, but really there's about 10 commits hiding in this one review by my quick count. It makes bisecting quite a bit simpler to do separate commits.

Yes, this started off as just the changes to stand/efi, but I couldn't test that without the rest - it wouldn't boot.

I can break it up.

Usually many of the smaller changes will go in quickly if there are any others the prove to need more discussion. So that can also narrow the focus quickly. Thanks.

In D51094#1168500, @imp wrote:

Usually many of the smaller changes will go in quickly if there are any others the prove to need more discussion. So that can also narrow the focus quickly. Thanks.

Hmm I can't easily break it up too much - else things will not compile.
D51186 covers the change to stand/efi

D51187 fixing secure netboot is the bulk of the other changes.

The changes to bootp.c and pkgfs.c can be skipped - just changing the debug printfs to the new macro - introduced above