• Kees Cook's avatar
    USB: serial: whiteheat: fix heap overflow in WHITEHEAT_GET_DTR_RTS · e23e50e7
    Kees Cook authored
    The sizeof(struct whitehat_dr_info) can be 4 bytes under CONFIG_AEABI=n
    due to "-mabi=apcs-gnu", even though it has a single u8:
    
    whiteheat_private {
            __u8                       mcr;                  /*     0     1 */
    
            /* size: 4, cachelines: 1, members: 1 */
            /* padding: 3 */
            /* last cacheline: 4 bytes */
    };
    
    The result is technically harmless, as both the source and the
    destinations are currently the same allocation size (4 bytes) and don't
    use their padding, but if anything were to ever be added after the
    "mcr" member in "struct whiteheat_private", it would be overwritten. The
    structs both have a single u8 "mcr" member, but are 4 bytes in padded
    size. The memcpy() destination was explicitly targeting the u8 member
    (size 1) with the length of the whole structure (size 4), triggering
    the memcpy buffer overflow warning:
    
    In file included from include/linux/string.h:253,
                     from include/linux/bitmap.h:11,
                     from include/linux/cpumask.h:12,
                     from include/linux/smp.h:13,
                     from include/linux/lockdep.h:14,
                     from include/linux/spinlock.h:62,
                     from include/linux/mmzone.h:8,
                     from include/linux/gfp.h:6,
                     from include/linux/slab.h:15,
                     from drivers/usb/serial/whiteheat.c:17:
    In function 'fortify_memcpy_chk',
        inlined from 'firm_send_command' at drivers/usb/serial/whiteheat.c:587:4:
    include/linux/fortify-string.h:328:25: warning: call to '__write_overflow_field' declared with attribute warning: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
      328 |                         __write_overflow_field(p_size_field, size);
          |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    Instead, just assign the one byte directly.
    Reported-by: default avatarkernel test robot <lkp@intel.com>
    Link: https://lore.kernel.org/lkml/202204142318.vDqjjSFn-lkp@intel.com
    Cc: stable@vger.kernel.org
    Signed-off-by: default avatarKees Cook <keescook@chromium.org>
    Link: https://lore.kernel.org/r/20220421001234.2421107-1-keescook@chromium.orgSigned-off-by: default avatarJohan Hovold <johan@kernel.org>
    e23e50e7
whiteheat.c 23.4 KB