Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 16 Jul 2021 18:30:46 GMT
From:      Warner Losh <imp@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 37053c21e9d9 - stable/13 - loader: support.4th resets the read buffer incorrectly
Message-ID:  <202107161830.16GIUkFE047450@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=37053c21e9d984e50d7cac0002f8ec370213c7b1

commit 37053c21e9d984e50d7cac0002f8ec370213c7b1
Author:     John Hood <cgull+l-freebsd-bugzilla@glup.org>
AuthorDate: 2021-07-11 14:44:12 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2021-07-16 18:28:44 +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.
    
    PR: 239315
    Reviewed by: imp
    
    (cherry picked from commit 9c1c02093b90ae49745a174eb26ea85dd1990eec)
---
 stand/forth/support.4th | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202107161830.16GIUkFE047450>