Page MenuHomeFreeBSD

Three fixes to the /boot/lua loader code.
Needs ReviewPublic

Authored by crahman_gmail.com on Jul 28 2022, 5:17 PM.

Details

Reviewers
kevans
imp
manu
Summary

Bug 265471 - loader: /boot/lua/menu.lua boots kernel from original bootenv
when bootenv is changed.

This is related to a bug fixed by Kyle Evans <kevans@FreeBSD.org> in commit
e414851f3eb7a9dd2af8209eac4ada307cb6ff8e:

If a password is set for the loader, when the boot environment is changed in
the lua menu, the kernel originally loaded before the menu was brought up is
booted in the new boot environment.

This fixes the problem by unloading any previously loaded kernel when
changing the boot environment.

If the problem isn't fixed, when a password is used for the loader's boot
menu, if an alternate bootenv is chosen from the menu, the original bootenv's
selected kernel will be booted but the new bootenv will be mounted on /.

The kernel's /boot/kernel directory won't be mounted, which will prevent
modules from being loaded during boot. In addition, if the reason for using
the boot environment is to provide a known good kernel and environment while
experimenting with a new kernel, the resulting confusion will be unhelpful.

The problem only occurs when a loader password is set in /boot/loader.conf. However
given that important and dangerous functions such as the restoration of a zpool
checkpoint are now in the boot menu. such a password has become important.

Bug 265472 - /boot/loader: lua autoboot timer restarts if menu password is set

In the lua loader, if one sets a password in loader.conf, the autoboot_delay
timer will start before the menu is displayed. One can interrupt the
autoboot and bring up the menu by entering a keyboard character before the
timer expires.

If this is done a prompt for the password is displayed. Entering the
password will bring up the menu, but the timer will again start and another
keyboard character must be entered or autoboot will abort the menu and boot
the system.

It's common to use a short timeout and enter a key before the password prompt
when you need to enter the menu. Having done this, if you are now studying
the menu trying to decide what to do, the system will autoboot before you do.

Bug 265001 - loader: Lines containing a comment that contains a '"' in
loader.conf are ignored.

A comment is an unexpected and difficult to find cause of breakage.

Consider the example /boot/loader.conf entry:
test_directive_b="test b" # This is "test_directive_b"

The problem is that the lua parser in /boot/lua/config.lua is splitting the
line on the '=', sending the first match to the key and the second to the
value.

In the problem case, the value becomes:

"test b"               # This is "test_directive_b"

Then this value gets matched against QVALEXPR = '"(.*)"'.

This cleans up the value by removing the first and last '"', but in this case
turns it into:

test b"               # This is "test_directive_b

which then gets rejected in processEnvVar() with MSG_FAILSYN_QUOTE, since
values aren't allowed to contain '"'.

Changing QVALEXPR to '([-%w_]+)' fixes this problem. Since the value is
explicitly prevented from containing quotes, it's not unreasonable to load it
from an expression that excludes them. The only other place QVALEXPR is used
is in the 'exec="command"' expression, which also should not contain '"' in
the command value.

Test Plan

The first two bugs and their fixes are best tested by setting a loader password in
/boot/loader.conf, e.g. 'password=testpass'.

To test the first bug, create a boot environment, place an alternate kernel in
/kernel in the new boot environment, and try booting it after selecting the
new boot environment from the loader's bootenv menu.

Note that if you do use the kernel selection menu after selecting the new boot
environment, the correct kernel will be booted. So, if you know about the
problem there is a workaround.

Testing the second bug and its fix can easily be done while testing the first bug
after setting a loader password.

To test the third bug and its fix, add the following lines to /boot/loader.conf:

test_directive_a="test a" # This is test_directive_a
test_directive_b="test b" # This is "test_directive_b"
test_directive_c="test c" # This is 'test_directive_b'

Boot a system and inspect the results, perhaps with kenv(1). The values
for "test b" will not be present.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 46659
Build 43548: arc lint + arc unit