@matheusbsbs / matheus-bagliano / commits / 48fb7deb5b

Fork 0

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 <torvalds@linux-foundation.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Linus Torvalds committed Jun 17, 2009 at 17:22 UTC 48fb7deb5bbd87933e7d314b73d7c1b52667f80f

8 files changed +12 -16

attr.c

+1 -2

@@ -35,8 +35,7 @@ static struct git_attr *(git_attr_hash[HASHSIZE]);

36 static unsigned hash_name(const char *name, int namelen)

37 {

38 - unsigned val = 0;

39 - unsigned char c;

38 + unsigned val = 0, c;

40 while (namelen--) {

41 c = *name++;

base85.c

+1 -1

@@ -91,7 +91,7 @@ void encode_85(char *buf, const unsigned char *data, int bytes)

91 unsigned acc = 0;

92 int cnt;

93 for (cnt = 24; cnt >= 0; cnt -= 8) {

94 - int ch = *data++;

94 + unsigned ch = *data++;

95 acc |= ch << cnt;

96 if (--bytes == 0)

97 break;

builtin-pack-objects.c

+1 -2

@@ -653,8 +653,7 @@ static void rehash_objects(void)

653

654 static unsigned name_hash(const char *name)

655 {

656 - unsigned char c;

657 - unsigned hash = 0;

656 + unsigned c, hash = 0;

657

658 if (!name)

659 return 0;

builtin-unpack-objects.c

+2 -2

@@ -422,8 +422,8 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,

422 static void unpack_one(unsigned nr)

423 {

424 unsigned shift;

425 - unsigned char *pack, c;

426 - unsigned long size;

425 + unsigned char *pack;

426 + unsigned long size, c;

427 enum object_type type;

428

429 obj_list[nr].offset = consumed_bytes;

delta.h

+2 -3

@@ -90,12 +90,11 @@ static inline unsigned long get_delta_hdr_size(const unsigned char **datap,

90 const unsigned char *top)

91 {

92 const unsigned char *data = *datap;

93 - unsigned char cmd;

94 - unsigned long size = 0;

93 + unsigned long cmd, size = 0;

94 int i = 0;

95 do {

96 cmd = *data++;

98 - size |= (cmd & ~0x80) << i;

97 + size |= (cmd & 0x7f) << i;

98 i += 7;

99 } while (cmd & 0x80 && data < top);

100 *datap = data;

index-pack.c

+3 -3

@@ -293,8 +293,8 @@ static void *unpack_entry_data(unsigned long offset, unsigned long size)

293

294 static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_base)

295 {

296 - unsigned char *p, c;

297 - unsigned long size;

296 + unsigned char *p;

297 + unsigned long size, c;

298 off_t base_offset;

299 unsigned shift;

300 void *data;

@@ -312,7 +312,7 @@ static void *unpack_raw_entry(struct object_entry *obj, union delta_base *delta_

312 p = fill(1);

313 c = *p;

314 use(1);

315 - size += (c & 0x7fUL) << shift;

315 + size += (c & 0x7f) << shift;

316 shift += 7;

317 }

318 obj->size = size;

patch-delta.c

+1 -1

@@ -44,7 +44,7 @@ void *patch_delta(const void *src_buf, unsigned long src_size,

44 if (cmd & 0x01) cp_off = *data++;

45 if (cmd & 0x02) cp_off |= (*data++ << 8);

46 if (cmd & 0x04) cp_off |= (*data++ << 16);

47 - if (cmd & 0x08) cp_off |= (*data++ << 24);

47 + if (cmd & 0x08) cp_off |= ((unsigned) *data++ << 24);

48 if (cmd & 0x10) cp_size = *data++;

49 if (cmd & 0x20) cp_size |= (*data++ << 8);

50 if (cmd & 0x40) cp_size |= (*data++ << 16);

sha1_file.c

+1 -2

@@ -1162,8 +1162,7 @@ unsigned long unpack_object_header_buffer(const unsigned char *buf,

1162 unsigned long len, enum object_type *type, unsigned long *sizep)

1163 {

1164 unsigned shift;

1165 - unsigned char c;

1166 - unsigned long size;

1165 + unsigned long size, c;

1166 unsigned long used = 0;

1167

1168 c = buf[used++];