diff mbox series

mt76: mt7921: fix kernel panic by accessing unallocated eeprom.data

Message ID c9e2a44da4daa00166c802a8c10527359358219d.1681158440.git.objelf@gmail.com (mailing list archive)
State New, archived
Headers show
Series mt76: mt7921: fix kernel panic by accessing unallocated eeprom.data | expand

Commit Message

Sean Wang April 10, 2023, 8:35 p.m. UTC
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(-)

Comments

kernel test robot April 10, 2023, 10:03 p.m. UTC | #1
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.3-rc6 next-20230406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/sean-wang-mediatek-com/mt76-mt7921-fix-kernel-panic-by-accessing-unallocated-eeprom-data/20230411-043801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link:    https://lore.kernel.org/r/c9e2a44da4daa00166c802a8c10527359358219d.1681158440.git.objelf%40gmail.com
patch subject: [PATCH] mt76: mt7921: fix kernel panic by accessing unallocated eeprom.data
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230411/202304110556.xbu5H3ka-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/9825ad75cf770d2a15f7008eca476121b9b3d794
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review sean-wang-mediatek-com/mt76-mt7921-fix-kernel-panic-by-accessing-unallocated-eeprom-data/20230411-043801
        git checkout 9825ad75cf770d2a15f7008eca476121b9b3d794
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/wireless/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202304110556.xbu5H3ka-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/wireless/mediatek/mt76/mt7921/mcu.c: In function 'mt7921_mcu_parse_eeprom':
>> drivers/net/wireless/mediatek/mt76/mt7921/mcu.c:22:40: warning: variable 'res' set but not used [-Wunused-but-set-variable]
      22 |         struct mt7921_mcu_eeprom_info *res;
         |                                        ^~~


vim +/res +22 drivers/net/wireless/mediatek/mt76/mt7921/mcu.c

23bdc5d8cadfc9 Ming Yen Hsieh   2022-09-07  18  
1c099ab44727c8 Sean Wang        2021-01-28  19  static int
1c099ab44727c8 Sean Wang        2021-01-28  20  mt7921_mcu_parse_eeprom(struct mt76_dev *dev, struct sk_buff *skb)
1c099ab44727c8 Sean Wang        2021-01-28  21  {
1c099ab44727c8 Sean Wang        2021-01-28 @22  	struct mt7921_mcu_eeprom_info *res;
1c099ab44727c8 Sean Wang        2021-01-28  23  
1c099ab44727c8 Sean Wang        2021-01-28  24  	if (!skb)
1c099ab44727c8 Sean Wang        2021-01-28  25  		return -EINVAL;
1c099ab44727c8 Sean Wang        2021-01-28  26  
fc6ee71a2a8f2d Lorenzo Bianconi 2022-06-20  27  	skb_pull(skb, sizeof(struct mt76_connac2_mcu_rxd));
1c099ab44727c8 Sean Wang        2021-01-28  28  
1c099ab44727c8 Sean Wang        2021-01-28  29  	res = (struct mt7921_mcu_eeprom_info *)skb->data;
1c099ab44727c8 Sean Wang        2021-01-28  30  
1c099ab44727c8 Sean Wang        2021-01-28  31  	return 0;
1c099ab44727c8 Sean Wang        2021-01-28  32  }
1c099ab44727c8 Sean Wang        2021-01-28  33
Lorenzo Bianconi April 11, 2023, 8:54 a.m. UTC | #2
> 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
>
Alexandre Mergnat April 12, 2023, 8:09 a.m. UTC | #3
On 10/04/2023 22:35, sean.wang@mediatek.com wrote:
> 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.

Reviewed-by: Alexandre Mergnat <amergnat@baylibre.com>
diff mbox series

Patch

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;
 }