Re: [RESEND] [PATCH] Endianness bug in index cache logic

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [RESEND] [PATCH] Endianness bug in index cache logic

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:56

Nathaniel W Filardo [off-list ref] writes:
quoted hunk
I got some free time and tracked it down.  The following one-line delta
fixes this issue for me; AIUI on sparc64 "unsigned long" is 8 bits and in
the wrong endianness for the memcpy trick to work as written?  I could be
sligntly off in my assessment of the problem, tho'.

index 1bbaf1c..9033dd3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1322,7 +1322,7 @@ int read_index_from(struct index_state *istate, const char *path)
                 * extension name (4-byte) and section length
                 * in 4-byte network byte order.
                 */
-               unsigned long extsize;
+               uint32_t extsize;
                memcpy(&extsize, (char *)mmap + src_offset + 4, 4);
                extsize = ntohl(extsize);
                if (read_index_extension(istate,
Good catch.

The original is broken on big endian 64-bit platforms, and your sparc64
indeed is one.  The code expects to see a signature (4-byte) at src_offset
followed by length (4-byte int in network byte order) and it is trying to
read read the latter in extsize.

Thanks for the fix.  The bug dates back to late April 2006, and I am kind
of surprised that nobody reported this since then (perhaps nobody runs git
on big endian 64-bit boxes?).

Re: [RESEND] [PATCH] Endianness bug in index cache logic

From: Tomas Carnecky <hidden>
Date: 2016-06-15 22:47:56

On 12/27/09 7:24 PM, Junio C Hamano wrote:
Nathaniel W Filardo[off-list ref]  writes:
quoted
I got some free time and tracked it down.  The following one-line delta
fixes this issue for me; AIUI on sparc64 "unsigned long" is 8 bits and in
the wrong endianness for the memcpy trick to work as written?  I could be
sligntly off in my assessment of the problem, tho'.

index 1bbaf1c..9033dd3 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1322,7 +1322,7 @@ int read_index_from(struct index_state *istate, const char *path)
                  * extension name (4-byte) and section length
                  * in 4-byte network byte order.
                  */
-               unsigned long extsize;
+               uint32_t extsize;
                 memcpy(&extsize, (char *)mmap + src_offset + 4, 4);
                 extsize = ntohl(extsize);
                 if (read_index_extension(istate,
Good catch.

The original is broken on big endian 64-bit platforms, and your sparc64
indeed is one.  The code expects to see a signature (4-byte) at src_offset
followed by length (4-byte int in network byte order) and it is trying to
read read the latter in extsize.

Thanks for the fix.  The bug dates back to late April 2006, and I am kind
of surprised that nobody reported this since then (perhaps nobody runs git
on big endian 64-bit boxes?).
Both the native Sun compiler as well as GCC default to 32bit binaries, 
even if the system is capable of running 64bit binaries (unlike for 
example Linux where x86_64-pc-linux-gnu-gcc produces 64bit binaries by 
default). And the git makefile doesn't use -m64, so my guess is that 
nobody bothered changing CFLAGS on sparc64 systems (I have access to a 
couple such systems and I never changed CFLAGS because git always worked 
out of the box). Though I don't know what the default is on other big 
endian systems.

tom
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help