Re: [PATCH] wifi: mt76: mt7921s: fix slab-out-of-bounds access in sdio host

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4133 bytes --]

> SDIO may need addtional 512 bytes to align bus operation. If the tailroom
> of this skb is not big enough, we would access invalid memory region.
> For low level operation, take xmit_buf instead of skb to keep valid memory
> access in SDIO.
> Note: xmit_buf is big enough for single skb size
> 
> Error message:
> [69.951] BUG: KASAN: slab-out-of-bounds in sg_copy_buffer+0xe9/0x1a0
> [69.951] Read of size 64 at addr ffff88811c9cf000 by task kworker/u16:7/451
> [69.951] CPU: 4 PID: 451 Comm: kworker/u16:7 Tainted: G W  OE  6.1.0-rc5 #1
> [69.951] Workqueue: kvub300c vub300_cmndwork_thread [vub300]
> [69.951] Call Trace:
> [69.951]  <TASK>
> [69.952]  dump_stack_lvl+0x49/0x63
> [69.952]  print_report+0x171/0x4a8
> [69.952]  kasan_report+0xb4/0x130
> [69.952]  kasan_check_range+0x149/0x1e0
> [69.952]  memcpy+0x24/0x70
> [69.952]  sg_copy_buffer+0xe9/0x1a0
> [69.952]  sg_copy_to_buffer+0x12/0x20
> [69.952]  __command_write_data.isra.0+0x23c/0xbf0 [vub300]
> [69.952]  vub300_cmndwork_thread+0x17f3/0x58b0 [vub300]
> [69.952]  process_one_work+0x7ee/0x1320
> [69.952]  worker_thread+0x53c/0x1240
> [69.952]  kthread+0x2b8/0x370
> [69.952]  ret_from_fork+0x1f/0x30
> [69.952]  </TASK>
> 
> [69.952] Allocated by task 854:
> [69.952]  kasan_save_stack+0x26/0x50
> [69.952]  kasan_set_track+0x25/0x30
> [69.952]  kasan_save_alloc_info+0x1b/0x30
> [69.952]  __kasan_kmalloc+0x87/0xa0
> [69.952]  __kmalloc_node_track_caller+0x63/0x150
> [69.952]  kmalloc_reserve+0x31/0xd0
> [69.952]  __alloc_skb+0xfc/0x2b0
> [69.952]  __mt76_mcu_msg_alloc+0xbf/0x230 [mt76]
> [69.952]  mt76_mcu_send_and_get_msg+0xab/0x110 [mt76]
> [69.952]  __mt76_mcu_send_firmware.cold+0x94/0x15d [mt76]
> [69.952]  mt76_connac_mcu_send_ram_firmware+0x415/0x54d [mt76_connac_lib]
> [69.952]  mt76_connac2_load_ram.cold+0x118/0x4bc [mt76_connac_lib]
> [69.952]  mt7921_run_firmware.cold+0x2e9/0x405 [mt7921_common]
> [69.952]  mt7921s_mcu_init+0x45/0x80 [mt7921s]
> [69.953]  mt7921_init_work+0xe1/0x2a0 [mt7921_common]
> [69.953]  process_one_work+0x7ee/0x1320
> [69.953]  worker_thread+0x53c/0x1240
> [69.953]  kthread+0x2b8/0x370
> [69.953]  ret_from_fork+0x1f/0x30
> [69.953] The buggy address belongs to the object at ffff88811c9ce800
>              which belongs to the cache kmalloc-2k of size 2048
> [69.953] The buggy address is located 0 bytes to the right of
>              2048-byte region [ffff88811c9ce800, ffff88811c9cf000)
> 
> [69.953] Memory state around the buggy address:
> [69.953]  ffff88811c9cef00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [69.953]  ffff88811c9cef80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [69.953] >ffff88811c9cf000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [69.953]                    ^
> [69.953]  ffff88811c9cf080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> [69.953]  ffff88811c9cf100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> 
> Fixes: 764dee47e2c1 ("mt76: sdio: move common code in mt76_sdio module")
> Tested-by: YN Chen <YN.Chen@mediatek.com>
> Signed-off-by: Deren Wu <deren.wu@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/sdio_txrx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index bfc4de50a4d2..ebea5c4e8da5 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -254,7 +254,8 @@ static int mt76s_tx_run_queue(struct mt76_dev *dev, struct mt76_queue *q)
>  
>  		if (!test_bit(MT76_STATE_MCU_RUNNING, &dev->phy.state)) {
>  			__skb_put_zero(e->skb, 4);
> -			err = __mt76s_xmit_queue(dev, e->skb->data,
> +			memcpy(sdio->xmit_buf, e->skb->data, e->skb->len);

(even if it is not critical for performance) iirc the skb from the mcu is
always linear, I guess we can use __skb_grow() instead. What do you think?

Regards,
Lorenzo

> +			err = __mt76s_xmit_queue(dev, sdio->xmit_buf,
>  						 e->skb->len);
>  			if (err)
>  				return err;
> -- 
> 2.18.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]