| Message ID | c9e2a44da4daa00166c802a8c10527359358219d.1681158440.git.objelf@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | mt76: mt7921: fix kernel panic by accessing unallocated eeprom.data | expand |
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
> 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 >
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 --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; }