From owner-dev-commits-src-branches@freebsd.org Fri Jul 16 18:30:47 2021 Return-Path: Delivered-To: dev-commits-src-branches@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 5F8DA665CD3; Fri, 16 Jul 2021 18:30:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GRKYf52zLz3vD2; Fri, 16 Jul 2021 18:30:46 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 5703921BC9; Fri, 16 Jul 2021 18:30:46 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 16GIUk2x047451; Fri, 16 Jul 2021 18:30:46 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 16GIUkFE047450; Fri, 16 Jul 2021 18:30:46 GMT (envelope-from git) Date: Fri, 16 Jul 2021 18:30:46 GMT Message-Id: <202107161830.16GIUkFE047450@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Warner Losh Subject: git: 37053c21e9d9 - stable/13 - loader: support.4th resets the read buffer incorrectly MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 37053c21e9d984e50d7cac0002f8ec370213c7b1 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-branches@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commits to the stable branches of the FreeBSD src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Jul 2021 18:30:47 -0000 The branch stable/13 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=37053c21e9d984e50d7cac0002f8ec370213c7b1 commit 37053c21e9d984e50d7cac0002f8ec370213c7b1 Author: John Hood AuthorDate: 2021-07-11 14:44:12 +0000 Commit: Warner Losh 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