Re: [PATCH] fsck: properly bound "invalid tag name" error message

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 8, 2014, 11:28 UTC
Message-ID: <20141208112835.GA15919@lanh>
In-Reply-To: <20141208055706.GA30207@peff.net>

On Mon, Dec 08, 2014 at 12:57:06AM -0500, Jeff King wrote:

> I do admit that I am tempted to teach index-pack to always NUL-terminate
> objects in memory that we feed to fsck, just to be on the safe side. It
> doesn't cost much, and could prevent a silly mistake (either in the
> future, or one that I missed in my analysis).

I think I'm missing a "but.." here. Maybe "but I didn't have time". The change looks simple enough. The remaining *alloc in index-pack is either for arrays, or already NUL-terminated (patch_delta), or does explicit boundary check (compare_objects).

It may be interesting to go over `git grep alloc\(` and see if we should use the allocz version instead. I think in some place we do xmalloc(len + 1) which could be replaced with xmallocz(len)

-- 8< --
Subject: [PATCH] index-pack: terminate object buffers with NUL

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/index-pack.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index a369f55..4632117 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -447,7 +447,7 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size,
 	if (type == OBJ_BLOB && size > big_file_threshold)
 		buf = fixed_buf;
 	else
-		buf = xmalloc(size);
+		buf = xmallocz(size);
 
 	memset(&stream, 0, sizeof(stream));
 	git_inflate_init(&stream);
@@ -552,7 +552,7 @@ static void *unpack_data(struct object_entry *obj,
 	git_zstream stream;
 	int status;
 
-	data = xmalloc(consume ? 64*1024 : obj->size);
+	data = xmallocz(consume ? 64*1024 : obj->size);
 	inbuf = xmalloc((len < 64*1024) ? len : 64*1024);
 
 	memset(&stream, 0, sizeof(stream));

-- 
2.2.0.60.gb7b3c64

-- 8< --