From 33b2e30284c915e952968051d6bdba4bb282f87e Mon Sep 17 00:00:00 2001 From: Sami Kerola Date: Sat, 12 May 2018 22:45:58 +0100 Subject: include/pt-mbr.h: fix integer overflow gcc -fsanitize=undefined gives following warning. include/pt-mbr.h:27:51: runtime error: left shift of 248 by 24 places cannot be represented in type 'int' It looks like char is converted internally to int before bit-shift, and that type overflows when char value is greater than 127. Following code snippet will show the effect what is stored when undefined behaviour happens. #include #include int main(int argc, unsigned char **argv) { char p[] = { 170, 170, 170, 170 }; unsigned int uint = p[3]; uint64_t res = 0; /* overflow */ res = p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24); printf("%" PRIu64 "\n", res); /* this is fine */ res = 0; res = p[0] | (p[1] << 8) | (p[2] << 16) | (uint << 24); printf("%" PRIu64 "\n", res); return 0; } I tested gcc 8.1.0, clang 6.0.0, and tcc 0.9.27 and they all printed the same values. $ ./a.out 18446744073709551530 4294967210 Because output is result of undefined behavior what is stored may change in future, and other compilers / version might do something different. In the case of what pt-mbr.h the destination data type size was commonly 32 bits in size, that truncated excess rubbish from bitshift. Needless to say that was not very robust code. Signed-off-by: Sami Kerola --- include/pt-mbr.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/pt-mbr.h b/include/pt-mbr.h index 1f265ed06..1a38246f1 100644 --- a/include/pt-mbr.h +++ b/include/pt-mbr.h @@ -22,9 +22,11 @@ static inline struct dos_partition *mbr_get_partition(unsigned char *mbr, int i) } /* assemble badly aligned little endian integer */ -static inline unsigned int __dos_assemble_4le(const unsigned char *p) +static inline uint32_t __dos_assemble_4le(const unsigned char *p) { - return p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24); + uint32_t last_byte = p[3]; + + return p[0] | (p[1] << 8) | (p[2] << 16) | (last_byte << 24); } static inline void __dos_store_4le(unsigned char *p, unsigned int val) -- cgit v1.2.3-55-g7522