On Sun, Oct 28, 2012 at 12:49:55AM +0200, Romain Francoise wrote:
AddressSanitizer (http://clang.llvm.org/docs/AddressSanitizer.html)
complains of a one-byte buffer underflow in parse_name_and_email() while
running the test suite. And indeed, if one of the lines in the mailmap
begins with '', we dereference the address just before the beginning of
the buffer when looking for whitespace to remove, before checking that
we aren't going too far.
So reverse the order of the tests to make sure that we don't read
outside the buffer.
Thanks, I think your fix is correct.
diff --git a/mailmap.c b/mailmap.c
index 47aa419..ea4b471 100644
--- a/mailmap.c
+++ b/mailmap.c
@@ -118,7 +118,7 @@ static char *parse_name_and_email(char *buffer, char
**name,
while (isspace(*nstart) nstart left)
++nstart;
nend = left-1;
- while (isspace(*nend) nend nstart)
+ while (nend nstart isspace(*nend))
--nend;
The fix confused me for a moment, because the problem is not actually in
the loop condition itself; working backwards from nend nstart, we
will at worst dereference nstart unnecessarily. The real problem is in
the nend = left-1 above, which sets the loop precondition outside the
string to be examined.
So you could also check for left == nstart before the loop even
begins. I think your fix (to just make the loop more robust to that
precondition) is better, though, as the rest of the code does the right
thing with such a value of nend.
It looks like t4203 triggers this problem. Curious that valgrind does
not find it. I guess since it does not have compiler support, it cannot
find out-of-bound errors on stack buffers. Does the rest of the test
suite turn up clean with AddressSanitizer?
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
