freebsd-nq/stand/forth
John Hood dbdf2b52f5 loader: support.4th resets the read buffer incorrectly
Large nextboot.conf files (over 80 bytes) are not read correctly by the
Forth loader, causing file parsing to abort, and nextboot configuration
fails to apply.

Simple repro:

nextboot -e foo=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
shutdown -r now

That will cause the bug to cause a parse failure but shouldn't otherwise
affect the boot.  Depending on your loader configuration, you may also
have to set beastie_disable and/or reduce the number of modules loaded
to see the error on a small console screen.  12.0 or CURRENT users will
also have to explicitly use the Forth loader instead of the Lua loader.
The error will look something like:

Warning: syntax error on file /boot/loader.conf.local
foo="xxxxxxxxxxxxxxnextboot_enable="YES"
                                    ^
/boot/support.4th has crude file I/O buffering, which uses a buffer
'read_buffer', defined to be 80 bytes by the 'read_buffer_size'
constant.  The loader first tastes nextboot.conf, reading and parsing
the first line in it for nextboot_enable="YES".  If this is true, then
it reopens the file and parses it like other loader .conf files.

Unfortunately, the file I/O buffering code does not fully reset the
buffer state in the reset_line_reading word.  If the last file was read
to the end, that doesn't matter; the file buffer is treated as empty
anyway.  But in the nextboot.conf case, the loader will not read to the
end of file if it is over 80 bytes, and the file buffer may be reused
when reading the next file.  When the file is reread, the corrupt text
may cause file parsing to abort on bad syntax (if the corrupt line has
<>2 quotes in it), the wrong variable to be set, no variable to be set
at all, or (if the splice happens to land at a line ending) something
approximating normal operation.

The bug is very old, dating back to at least 2000 if not before, and is
still present in 12.0 and CURRENT r345863 (though it is now hidden by
the Lua loader by default).

Suggested one-line attached.  This does change the behavior of the
reset_line_reading word, which is exported in the line-reading
dictionary (though the export is not documented in loader man pages).
But repo history shows it was probably exported for the PNP support
code, which was never included in the loader build, and was removed 5
months ago.

One thing that puzzles me: how has this bug gone unnoticed/unfixed for
nearly 2 decades?  I find it hard to believe that nobody's tried to do
something interesting with nextboot, like load a kernel and filesystem,
which is what I'm doing.

Tested by:		Gary Jennejohn
PR:			239315
MFC After:		3 weeks
Reviewed by:		imp (and correctly applied this time)
Differential Revision:	https://reviews.freebsd.org/D31328
2021-07-28 13:50:38 -06:00
..
beastie.4th
beastie.4th.8
brand-fbsd.4th
brand.4th
brand.4th.8
check-password.4th
check-password.4th.8
color.4th
color.4th.8
delay.4th
delay.4th.8
efi.4th
frames.4th
loader.4th
loader.4th.8
loader.rc
logo-beastie.4th
logo-beastiebw.4th
logo-fbsdbw.4th
logo-orb.4th
logo-orbbw.4th
Makefile
Makefile.depend
menu-commands.4th
menu.4th
menu.4th.8
menu.rc
menusets.4th
menusets.4th.8
screen.4th
shortcuts.4th
support.4th
version.4th
version.4th.8