Zekun Shen <bruceshenzk@xxxxxxxxx> writes: > We found this bug while fuzzing the rsi_usb driver. > rsi_prepare_skb does not check for OOB memcpy. We > add the check in the caller to fix. > > Although rsi_prepare_skb checks if length is larger > than (4 * RSI_RCV_BUFFER_LEN), it really can't because > length is 0xfff maximum. So the check in patch is sufficient. > > This patch is created upon ath-next branch. It is > NOT tested with real device, but with QEMU emulator. > > Following is the bug report > > BUG: KASAN: use-after-free in rsi_read_pkt > (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > Read of size 3815 at addr ffff888031da736d by task RX-Thread/204 > > CPU: 0 PID: 204 Comm: RX-Thread Not tainted 5.6.0 #5 > Call Trace: > dump_stack (/linux/lib/dump_stack.c:120) > ? rsi_read_pkt (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > print_address_description.constprop.6 (/linux/mm/kasan/report.c:377) > ? rsi_read_pkt (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > ? rsi_read_pkt (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > __kasan_report.cold.9 (/linux/mm/kasan/report.c:510) > ? syscall_return_via_sysret (/linux/arch/x86/entry/entry_64.S:253) > ? rsi_read_pkt (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > kasan_report (/linux/arch/x86/include/asm/smap.h:69 /linux/mm/kasan/common.c:644) > check_memory_region (/linux/mm/kasan/generic.c:186 /linux/mm/kasan/generic.c:192) > memcpy (/linux/mm/kasan/common.c:130) > rsi_read_pkt (/linux/drivers/net/wireless/rsi/rsi_91x_main.c:206) rsi_91x > ? skb_dequeue (/linux/net/core/skbuff.c:3042) > rsi_usb_rx_thread (/linux/drivers/net/wireless/rsi/rsi_91x_usb_ops.c:47) rsi_usb > > Reported-by: Brendan Dolan-Gavitt <brendandg@xxxxxxx> > Signed-off-by: Zekun Shen <bruceshenzk@xxxxxxxxx> > --- > drivers/net/wireless/rsi/rsi_91x_main.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/wireless/rsi/rsi_91x_main.c b/drivers/net/wireless/rsi/rsi_91x_main.c > index 5d1490fc3..41d3c12e0 100644 > --- a/drivers/net/wireless/rsi/rsi_91x_main.c > +++ b/drivers/net/wireless/rsi/rsi_91x_main.c > @@ -193,6 +193,10 @@ int rsi_read_pkt(struct rsi_common *common, u8 *rx_pkt, s32 rcv_pkt_len) > break; > > case RSI_WIFI_DATA_Q: > + if (!rcv_pkt_len && offset + length > > + RSI_MAX_RX_USB_PKT_SIZE) > + goto fail; > + > skb = rsi_prepare_skb(common, > (frame_desc + offset), > length, Why are you doing the check here? In the beginning of the function we have: frame_desc = &rx_pkt[index]; actual_length = *(u16 *)&frame_desc[0]; offset = *(u16 *)&frame_desc[2]; if (!rcv_pkt_len && offset > RSI_MAX_RX_USB_PKT_SIZE - FRAME_DESC_SZ) goto fail; Wouldn't it make more sense to fix that check? -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
