git: 3a4b9e30d411 - stable/13 - loader: support.4th resets the read buffer incorrectly

[Warner Losh]

The branch stable/13 has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=3a4b9e30d4119b952fd0690cb7ab8eb9c4346317

commit 3a4b9e30d4119b952fd0690cb7ab8eb9c4346317
Author:     John Hood <cgull+l-freebsd-bugzilla at glup.org>
AuthorDate: 2021-07-28 19:43:02 +0000
Commit:     Warner Losh <imp at FreeBSD.org>
CommitDate: 2021-09-12 15:56:15 +0000

    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
    
    (cherry picked from commit dbdf2b52f59df7374eb1f799b4df1b54e4502e40)
---
 stand/forth/support.4th | 1 +
 1 file changed, 1 insertion(+)

diff --git a/stand/forth/support.4th b/stand/forth/support.4th
index d87cf16a16dd..999ac5005f5d 100644
--- a/stand/forth/support.4th
+++ b/stand/forth/support.4th
@@ -486,6 +486,7 @@ get-current ( -- wid ) previous definitions >search ( wid -- )
 
 : reset_line_reading
   0 to read_buffer_ptr
+  0 read_buffer .len !
 ;
 
 : read_line