Module Name: src
Committed By: rillig
Date: Tue Dec 22 08:05:08 UTC 2020
Modified Files:
src/usr.bin/make: parse.c
src/usr.bin/make/unit-tests: opt-file.exp opt-file.mk
Log Message:
make(1): fix assertion failure for files without trailing newline
Previously, mmapped files didn't always have the final newline added.
Only those that ended at a page boundary did.
This confused ParseRawLine, which assumed (and since parse.c 1.510 from
moments ago also asserted) that every line ends with a newline, which
allows the code to assume that after a backslash, there is at least one
other character in the buffer, thereby preventing an out-of-bounds read.
This bug had been there at least since parse.c 1.170 from 2010-12-25
04:57:07, maybe even earlier, I didn't check.
Now line_end always points to the trailing newline, which allows
ParseGetLine to overwrite that character to end the string.
To generate a diff of this commit:
cvs rdiff -u -r1.510 -r1.511 src/usr.bin/make/parse.c
cvs rdiff -u -r1.5 -r1.6 src/usr.bin/make/unit-tests/opt-file.exp
cvs rdiff -u -r1.7 -r1.8 src/usr.bin/make/unit-tests/opt-file.mk
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/usr.bin/make/parse.c
diff -u src/usr.bin/make/parse.c:1.510 src/usr.bin/make/parse.c:1.511
--- src/usr.bin/make/parse.c:1.510 Tue Dec 22 06:48:33 2020
+++ src/usr.bin/make/parse.c Tue Dec 22 08:05:08 2020
@@ -1,4 +1,4 @@
-/* $NetBSD: parse.c,v 1.510 2020/12/22 06:48:33 rillig Exp $ */
+/* $NetBSD: parse.c,v 1.511 2020/12/22 08:05:08 rillig Exp $ */
/*
* Copyright (c) 1988, 1989, 1990, 1993
@@ -117,7 +117,7 @@
#include "pathnames.h"
/* "@(#)parse.c 8.3 (Berkeley) 3/19/94" */
-MAKE_RCSID("$NetBSD: parse.c,v 1.510 2020/12/22 06:48:33 rillig Exp $");
+MAKE_RCSID("$NetBSD: parse.c,v 1.511 2020/12/22 08:05:08 rillig Exp $");
/* types and constants */
@@ -466,13 +466,14 @@ loadedfile_mmap(struct loadedfile *lf, i
if (lf->buf == MAP_FAILED)
return FALSE;
- if (lf->len == lf->maplen && lf->buf[lf->len - 1] != '\n') {
- char *b = bmake_malloc(lf->len + 1);
- b[lf->len] = '\n';
- memcpy(b, lf->buf, lf->len++);
- munmap(lf->buf, lf->maplen);
- lf->maplen = 0;
- lf->buf = b;
+ if (lf->len > 0 && lf->buf[lf->len - 1] != '\n') {
+ if (lf->len == lf->maplen) {
+ char *b = bmake_malloc(lf->len + 1);
+ memcpy(b, lf->buf, lf->len);
+ munmap(lf->buf, lf->maplen);
+ lf->maplen = 0;
+ }
+ lf->buf[lf->len++] = '\n';
}
return TRUE;
@@ -2687,19 +2688,15 @@ ParseRawLine(IFile *curFile, char **out_
if (ch == '\\') {
if (firstBackslash == NULL)
firstBackslash = p;
- /*
- * FIXME: In opt-file.mk, this command succeeds:
- * printf '%s' 'V=v\' | make -r -f -
- * Using an intermediate file fails though:
- * printf '%s' 'V=v\' > backslash
- * make -r -f backslash
- *
- * In loadedfile_mmap, the trailing newline is not
- * added in every case, only if the file ends at a
- * page boundary.
- */
- if (p[1] == '\n')
+ if (p[1] == '\n') {
curFile->lineno++;
+ if (p + 2 == curFile->buf_end) {
+ line_end = p;
+ *line_end = '\n';
+ p += 2;
+ continue;
+ }
+ }
p += 2;
line_end = p;
assert(p <= curFile->buf_end);
@@ -2843,12 +2840,8 @@ ParseGetLine(GetLineMode mode)
}
/* We now have a line of data */
- /*
- * FIXME: undefined behavior since line_end points right
- * after the allocated buffer. This becomes apparent when
- * using a strict malloc implementation that adds canaries
- * before and after the allocated space.
- */
+ /* TODO: Remove line_end, it's not necessary here. */
+ assert(*line_end == '\n');
*line_end = '\0';
if (mode == GLM_FOR_BODY)
Index: src/usr.bin/make/unit-tests/opt-file.exp
diff -u src/usr.bin/make/unit-tests/opt-file.exp:1.5 src/usr.bin/make/unit-tests/opt-file.exp:1.6
--- src/usr.bin/make/unit-tests/opt-file.exp:1.5 Mon Dec 7 00:53:30 2020
+++ src/usr.bin/make/unit-tests/opt-file.exp Tue Dec 22 08:05:08 2020
@@ -1,4 +1,5 @@
value
+value
make: "(stdin)" line 1: Zero byte read from file
make: Fatal errors encountered -- cannot continue
make: stopped in unit-tests
Index: src/usr.bin/make/unit-tests/opt-file.mk
diff -u src/usr.bin/make/unit-tests/opt-file.mk:1.7 src/usr.bin/make/unit-tests/opt-file.mk:1.8
--- src/usr.bin/make/unit-tests/opt-file.mk:1.7 Sun Dec 6 20:55:30 2020
+++ src/usr.bin/make/unit-tests/opt-file.mk Tue Dec 22 08:05:08 2020
@@ -1,4 +1,4 @@
-# $NetBSD: opt-file.mk,v 1.7 2020/12/06 20:55:30 rillig Exp $
+# $NetBSD: opt-file.mk,v 1.8 2020/12/22 08:05:08 rillig Exp $
#
# Tests for the -f command line option.
@@ -6,6 +6,7 @@
all: .PHONY
all: file-ending-in-backslash
+all: file-ending-in-backslash-mmap
all: file-containing-null-byte
# Passing '-' as the filename reads from stdin. This is unusual but possible.
@@ -35,6 +36,14 @@ file-ending-in-backslash: .PHONY
@printf '%s' 'VAR=value\' \
| ${MAKE} -r -f - -V VAR
+# Between parse.c 1.170 from 2010-12-25 and parse.c 1.511 from 2020-12-22,
+# there was an out-of-bounds write in ParseGetLine, where line_end pointed at
+# the end of the allocated buffer, in the special case where loadedfile_mmap
+# had not added the final newline character.
+file-ending-in-backslash-mmap: .PHONY
+ @printf '%s' 'VAR=value\' > opt-file-backslash
+ @${MAKE} -r -f opt-file-backslash -V VAR
+
# If a file contains null bytes, the rest of the line is skipped, and parsing
# continues in the next line. Throughout the history of make, the behavior
# has changed several times, sometimes knowingly, sometimes by accident.
