]> git.ipfire.org Git - thirdparty/util-linux.git/commitdiff
include/pt-mbr.h: fix integer overflow
authorSami Kerola <kerolasa@iki.fi>
Sat, 12 May 2018 21:45:58 +0000 (22:45 +0100)
committerKarel Zak <kzak@redhat.com>
Mon, 28 May 2018 11:36:38 +0000 (13:36 +0200)
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 <stdio.h>
    #include <inttypes.h>
    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 <kerolasa@iki.fi>
include/pt-mbr.h

index 1f265ed06126959cf3369bfbf632df3c8d3c8536..1a38246f19294058c648faaac30954a998fcb613 100644 (file)
@@ -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)