threads / patch / 46262

patchpack-bitmap: Don't perform unaligned memory access

Subject: [PATCH] pack-bitmap: Don't perform unaligned memory access

## tl;dr

3 messages between Jun 26, 2017 and Jun 27, 2017. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

James Clarke· Jun 26, 2017, 15:16 UTC · lore

The preceding bitmap entries have a 1-byte XOR-offset and 1-byte flags, so their size is not a multiple of 4. Thus the name-hash cache is only guaranteed to be 2-byte aligned and so we must use get_be32 rather than indexing the array directly.

Signed-off-by: James Clarke <jrtc27@jrtc27.com>
---

This was noticed thanks to the recent tests added to t5310-pack-bitmaps.sh, which were crashing with SIGBUS on Debian sparc64. All tests (excluding those marked with known breakage) now pass again.

 pack-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Show changes to pack-bitmap.c +1 −2
diff --git a/pack-bitmap.c b/pack-bitmap.c
index a3ac3dccd..327634cd7 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -627,7 +627,7 @@ static void show_objects_for_type(
 			sha1 = nth_packed_object_sha1(bitmap_git.pack, entry->nr);

 			if (bitmap_git.hashes)
-				hash = ntohl(bitmap_git.hashes[entry->nr]);
+				hash = get_be32(bitmap_git.hashes + entry->nr);

 			show_reach(sha1, object_type, 0, hash, bitmap_git.pack, entry->offset);
 		}
--
2.13.2
Junio C Hamano· Jun 26, 2017, 19:30 UTC · re: James Clarke · lore

Re: [PATCH] pack-bitmap: Don't perform unaligned memory access

James Clarke <jrtc27@jrtc27.com> writes:
Show 11 quoted lines
> The preceding bitmap entries have a 1-byte XOR-offset and 1-byte flags,
> so their size is not a multiple of 4. Thus the name-hash cache is only
> guaranteed to be 2-byte aligned and so we must use get_be32 rather than
> indexing the array directly.
>
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
>
> This was noticed thanks to the recent tests added to t5310-pack-bitmaps.sh,
> which were crashing with SIGBUS on Debian sparc64. All tests (excluding those
> marked with known breakage) now pass again.
Thanks.
Show 19 quoted lines
>
>  pack-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/pack-bitmap.c b/pack-bitmap.c
> index a3ac3dccd..327634cd7 100644
> --- a/pack-bitmap.c
> +++ b/pack-bitmap.c
> @@ -627,7 +627,7 @@ static void show_objects_for_type(
>  			sha1 = nth_packed_object_sha1(bitmap_git.pack, entry->nr);
>
>  			if (bitmap_git.hashes)
> -				hash = ntohl(bitmap_git.hashes[entry->nr]);
> +				hash = get_be32(bitmap_git.hashes + entry->nr);
>
>  			show_reach(sha1, object_type, 0, hash, bitmap_git.pack, entry->offset);
>  		}
> --
> 2.13.2
Jonathan Nieder· Jun 27, 2017, 00:06 UTC · re: James Clarke · lore

Re: [PATCH] pack-bitmap: Don't perform unaligned memory access

James Clarke wrote:
Show 9 quoted lines
> The preceding bitmap entries have a 1-byte XOR-offset and 1-byte flags,
> so their size is not a multiple of 4. Thus the name-hash cache is only
> guaranteed to be 2-byte aligned and so we must use get_be32 rather than
> indexing the array directly.
>
> Signed-off-by: James Clarke <jrtc27@jrtc27.com>
> ---
>  pack-bitmap.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Failing build log: https://buildd.debian.org/status/fetch.php?pkg=git&arch=sparc64&ver=1%3A2.13.2-2&stamp=1498520310&raw=0

Thanks for tracking down and fixing it.

← back to recent threads