Re: [PATCH] mt76: mt7921: fix kernel panic by accessing unallocated eeprom.data

[Lorenzo Bianconi <lorenzo@kernel.org>]

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

> From: Sean Wang <sean.wang@mediatek.com>
> 
> The MT7921 driver no longer uses eeprom.data, but the relevant code has not
> been removed completely since
> commit 16d98b548365 ("mt76: mt7921: rely on mcu_get_nic_capability").
> This could result in potential invalid memory access.
> 
> To fix the kernel panic issue in mt7921, it is necessary to avoid accessing
> unallocated eeprom.data which can lead to invalid memory access.
> 
> [2.702735] BUG: kernel NULL pointer dereference, address: 0000000000000550
> [2.702740] #PF: supervisor write access in kernel mode
> [2.702741] #PF: error_code(0x0002) - not-present page
> [2.702743] PGD 0 P4D 0
> [2.702747] Oops: 0002 [#1] PREEMPT SMP NOPTI
> [2.702755] RIP: 0010:mt7921_mcu_parse_response+0x147/0x170 [mt7921_common]
> [2.702758] RSP: 0018:ffffae7c00fef828 EFLAGS: 00010286
> [2.702760] RAX: ffffa367f57be024 RBX: ffffa367cc7bf500 RCX: 0000000000000000
> [2.702762] RDX: 0000000000000550 RSI: 0000000000000000 RDI: ffffa367cc7bf500
> [2.702763] RBP: ffffae7c00fef840 R08: ffffa367cb167000 R09: 0000000000000005
> [2.702764] R10: 0000000000000000 R11: ffffffffc04702e4 R12: ffffa367e8329f40
> [2.702766] R13: 0000000000000000 R14: 0000000000000001 R15: ffffa367e8329f40
> [2.702768] FS:  000079ee6cf20c40(0000) GS:ffffa36b2f940000(0000) knlGS:0000000000000000
> [2.702769] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [2.702775] CR2: 0000000000000550 CR3: 00000001233c6004 CR4: 0000000000770ee0
> [2.702776] PKRU: 55555554
> [2.702777] Call Trace:
> [2.702782]  mt76_mcu_skb_send_and_get_msg+0xc3/0x11e [mt76 <HASH:1bc4 5>]
> [2.702785]  mt7921_run_firmware+0x241/0x853 [mt7921_common <HASH:6a2f 6>]
> [2.702789]  mt7921e_mcu_init+0x2b/0x56 [mt7921e <HASH:d290 7>]
> [2.702792]  mt7921_register_device+0x2eb/0x5a5 [mt7921_common <HASH:6a2f 6>]
> [2.702795]  ? mt7921_irq_tasklet+0x1d4/0x1d4 [mt7921e <HASH:d290 7>]
> [2.702797]  mt7921_pci_probe+0x2d6/0x319 [mt7921e <HASH:d290 7>]
> [2.702799]  pci_device_probe+0x9f/0x12a
> 
> Fixes: 16d98b548365 ("mt76: mt7921: rely on mcu_get_nic_capability")
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/net/wireless/mediatek/mt76/mt7921/mcu.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> index c5e7ad06f877..00c84680c723 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
> @@ -20,7 +20,6 @@ static int
>  mt7921_mcu_parse_eeprom(struct mt76_dev *dev, struct sk_buff *skb)
>  {
>  	struct mt7921_mcu_eeprom_info *res;
> -	u8 *buf;
>  
>  	if (!skb)
>  		return -EINVAL;
> @@ -28,8 +27,6 @@ mt7921_mcu_parse_eeprom(struct mt76_dev *dev, struct sk_buff *skb)
>  	skb_pull(skb, sizeof(struct mt76_connac2_mcu_rxd));
>  
>  	res = (struct mt7921_mcu_eeprom_info *)skb->data;
> -	buf = dev->eeprom.data + le32_to_cpu(res->addr);
> -	memcpy(buf, res->data, 16);
>  
>  	return 0;

I think we can just get rid of mt7921_mcu_parse_eeprom() here and use 'else'
branch in mt7921_mcu_parse_response() since now we just perform skb_pull().
Agree?

Regards,
Lorenzo

>  }
> -- 
> 2.25.1
> 

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