From 48fb7deb5bbd87933e7d314b73d7c1b52667f80f Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Wed, 17 Jun 2009 17:22:27 -0700 Subject: Fix big left-shifts of unsigned char Shifting 'unsigned char' or 'unsigned short' left can result in sign extension errors, since the C integer promotion rules means that the unsigned char/short will get implicitly promoted to a signed 'int' due to the shift (or due to other operations). This normally doesn't matter, but if you shift things up sufficiently, it will now set the sign bit in 'int', and a subsequent cast to a bigger type (eg 'long' or 'unsigned long') will now sign-extend the value despite the original expression being unsigned. One example of this would be something like unsigned long size; unsigned char c; size += c << 24; where despite all the variables being unsigned, 'c << 24' ends up being a signed entity, and will get sign-extended when then doing the addition in an 'unsigned long' type. Since git uses 'unsigned char' pointers extensively, we actually have this bug in a couple of places. I may have missed some, but this is the result of looking at git grep '[^0-9 ][ ]*<<[ ][a-z]' -- '*.c' '*.h' git grep '<<[ ]*24' which catches at least the common byte cases (shifting variables by a variable amount, and shifting by 24 bits). I also grepped for just 'unsigned char' variables in general, and converted the ones that most obviously ended up getting implicitly cast immediately anyway (eg hash_name(), encode_85()). In addition to just avoiding 'unsigned char', this patch also tries to use a common idiom for the delta header size thing. We had three different variations on it: "& 0x7fUL" in one place (getting the sign extension right), and "& ~0x80" and "& 0x7f" in two other places (not getting it right). Apart from making them all just avoid using "unsigned char" at all, I also unified them to then use a simple "& 0x7f". I considered making a sparse extension which warns about doing implicit casts from unsigned types to signed types, but it gets rather complex very quickly, so this is just a hack. Signed-off-by: Linus Torvalds Signed-off-by: Junio C Hamano --- patch-delta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'patch-delta.c') diff --git a/patch-delta.c b/patch-delta.c index ed9db81fa8..ef748ce96d 100644 --- a/patch-delta.c +++ b/patch-delta.c @@ -44,7 +44,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size, if (cmd & 0x01) cp_off = *data++; if (cmd & 0x02) cp_off |= (*data++ << 8); if (cmd & 0x04) cp_off |= (*data++ << 16); - if (cmd & 0x08) cp_off |= (*data++ << 24); + if (cmd & 0x08) cp_off |= ((unsigned) *data++ << 24); if (cmd & 0x10) cp_size = *data++; if (cmd & 0x20) cp_size |= (*data++ << 8); if (cmd & 0x40) cp_size |= (*data++ << 16); -- cgit v1.2.1 From 03aa8ff3be3b35522b2e378651e65e0e86778018 Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Mon, 14 Sep 2009 02:41:16 -0400 Subject: Nicolas Pitre has a new email address Due to problems at cam.org, my nico@cam.org email address is no longer valid. From now on, nico@fluxnic.net should be used instead. Signed-off-by: Nicolas Pitre Signed-off-by: Junio C Hamano --- patch-delta.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'patch-delta.c') diff --git a/patch-delta.c b/patch-delta.c index ef748ce96d..e02e13bd4e 100644 --- a/patch-delta.c +++ b/patch-delta.c @@ -2,7 +2,7 @@ * patch-delta.c: * recreate a buffer from a source and the delta produced by diff-delta.c * - * (C) 2005 Nicolas Pitre + * (C) 2005 Nicolas Pitre * * This code is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as -- cgit v1.2.1 From 222083a1585c058fd2bbcb76db1ea824ee3df17f Mon Sep 17 00:00:00 2001 From: Ilari Liusvaara Date: Tue, 26 Jan 2010 20:24:13 +0200 Subject: Fix integer overflow in patch_delta() Signed-off-by: Ilari Liusvaara Signed-off-by: Junio C Hamano --- patch-delta.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'patch-delta.c') diff --git a/patch-delta.c b/patch-delta.c index e02e13bd4e..d218faa02b 100644 --- a/patch-delta.c +++ b/patch-delta.c @@ -33,8 +33,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size, /* now the result size */ size = get_delta_hdr_size(&data, top); - dst_buf = xmalloc(size + 1); - dst_buf[size] = 0; + dst_buf = xmallocz(size); out = dst_buf; while (data < top) { -- cgit v1.2.1