git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/4] xdiff: avoid computing non-zero offset from NULL pointer

From
Jeff King <peff@peff.net>
Date
Jan 25, 2020, 05:39 UTC
Message-ID
<20200125053929.GC744673@coredump.intra.peff.net>
In-Reply-To
<20200125053542.GA744596@coredump.intra.peff.net>

As with the previous commit, clang-11's UBSan complains about computing offsets from a NULL pointer, causing some tests to fail. In this case, though, we're actually computing a non-zero offset, which is even more dubious. From t7810:

  xdiff-interface.c:268:14: runtime error: applying non-zero offset 1 to null pointer
  ...
  not ok 131 - grep -p with userdiff

The problem is our parsing of the funcname config. We count the number of lines in the string, allocate an array, and then loop over our allocated entries, parsing each line and moving our cursor to one past the trailing newline for the next iteration.

But the final line will not generally have a trailing newline (since it's a config value), and hence we go to one past NULL. In practice this is OK, since our loop should terminate before we look at the value. But even computing such an invalid pointer technically violates the standard.

We can fix it by leaving the pointer at NULL if we're at the end, rather than one-past. And while we're thinking about it, we can also document the variant by asserting that our initial line-count matches the second-pass of parsing.

Signed-off-by: Jeff King <peff@peff.net>
---
 xdiff-interface.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/xdiff-interface.c b/xdiff-interface.c
index 2f1fe48512..3cd2ac2855 100644
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -250,9 +250,13 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 	ALLOC_ARRAY(regs->array, regs->nr);
 	for (i = 0; i < regs->nr; i++) {
 		struct ff_reg *reg = regs->array + i;
-		const char *ep = strchr(value, '\n'), *expression;
+		const char *ep, *expression;
 		char *buffer = NULL;
 
+		if (!value)
+			BUG("mismatch between line count and parsing");
+		ep = strchr(value, '\n');
+
 		reg->negate = (*value == '!');
 		if (reg->negate && i == regs->nr - 1)
 			die("Last expression must not be negated: %s", value);
@@ -265,7 +269,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *value, int cflags)
 		if (regcomp(&reg->re, expression, cflags))
 			die("Invalid regexp to look for hunk header: %s", expression);
 		free(buffer);
-		value = ep + 1;
+		value = ep ? ep + 1 : NULL;
 	}
 }
 
-- 
2.25.0.421.gb74d19af79
Previous: Jeff KingNext: Jeff King
Message 15 of 17 in “more clang/sanitizer fixes”
  1. 0/4 more clang/sanitizer fixesJeff King, Jan 25, 2020
  2. 1/4 merge-recursive: silence -Wxor-used-as-pow warningJeff King, Jan 25, 2020
  3. Junio C HamanoJan 25, 2020
  4. Jeff KingJan 25, 2020
  5. Elijah NewrenJan 25, 2020
  6. Jeff KingJan 25, 2020
  7. Junio C HamanoJan 27, 2020
  8. 2/4 avoid computing zero offsets from NULL pointerJeff King, Jan 25, 2020
  9. Junio C HamanoJan 27, 2020
  10. Jeff KingJan 27, 2020
  11. Junio C HamanoJan 28, 2020
  12. Jeff KingJan 29, 2020
  13. Junio C HamanoJan 29, 2020
  14. Jeff KingJan 29, 2020
  15. 3/4 xdiff: avoid computing non-zero offset from NULL pointerJeff King, Jan 25, 2020
  16. 4/4 obstack: avoid computing offsets from NULL pointerJeff King, Jan 25, 2020
  17. 4/4 obstack: avoid computing offsets from NULL pointerJeff King, Jan 25, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.