Skip to content

Commit e23e50e

Browse files
keesjhovold
authored andcommitted
USB: serial: whiteheat: fix heap overflow in WHITEHEAT_GET_DTR_RTS
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: kernel test robot <[email protected]> Link: https://lore.kernel.org/lkml/[email protected] Cc: [email protected] Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Johan Hovold <[email protected]>
1 parent 35a923a commit e23e50e

File tree

1 file changed

+2
-3
lines changed

1 file changed

+2
-3
lines changed

drivers/usb/serial/whiteheat.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -584,9 +584,8 @@ static int firm_send_command(struct usb_serial_port *port, __u8 command,
584584
switch (command) {
585585
case WHITEHEAT_GET_DTR_RTS:
586586
info = usb_get_serial_port_data(port);
587-
memcpy(&info->mcr, command_info->result_buffer,
588-
sizeof(struct whiteheat_dr_info));
589-
break;
587+
info->mcr = command_info->result_buffer[0];
588+
break;
590589
}
591590
}
592591
exit:

0 commit comments

Comments
 (0)