Re: [PATCH v3 2/8] staging: qlge: Initialize devlink health dump framework

From: Coiby Xu <coiby.xu@gmail.com>
To: Shung-Hsi Yu <shung-hsi.yu@suse.com>
Cc: devel@driverdev.osuosl.org,
	Willem de Bruijn <willemdebruijn.kernel@gmail.com>,
	"supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER"
	<GR-Linux-NIC-Dev@marvell.com>,
	Manish Chopra <manishc@marvell.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	open list <linux-kernel@vger.kernel.org>,
	Benjamin Poirier <benjamin.poirier@gmail.com>,
	"open list:QLOGIC QLGE 10Gb ETHERNET DRIVER"
	<netdev@vger.kernel.org>,
	Dan Carpenter <dan.carpenter@oracle.com>
Subject: Re: [PATCH v3 2/8] staging: qlge: Initialize devlink health dump framework
Date: Tue, 20 Oct 2020 18:46:06 +0800	[thread overview]
Message-ID: <20201020104606.swblxgvfnjosafkk@Rk> (raw)
In-Reply-To: <20201020102741.GE23594@syu-laptop>

Hi,

Thank you for testing this patch!

On Tue, Oct 20, 2020 at 06:27:41PM +0800, Shung-Hsi Yu wrote:
>On Tue, Oct 20, 2020 at 04:57:11PM +0800, Shung-Hsi Yu wrote:
>> Hi,
>>
>> This patch trigger the following KASAN error inside qlge_init_device().
>>
>> [...] general protection fault, probably for non-canonical address 0xdffffc000000004b: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
>> [...] KASAN: null-ptr-deref in range [0x0000000000000258-0x000000000000025f]
>> [...] CPU: 0 PID: 438 Comm: systemd-udevd Tainted: G         C  E     5.9.0-kvmsmall+ #7
>> [...] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-48-
>> g...ilt.opensuse.org 04/01/2014
>> [...] RIP: 0010:qlge_get_8000_flash_params+0x377/0x6e0 [qlge]
>> [...] Code: 03 80 3c 02 00 0f 85 57 03 00 00 49 8b af 68 08 00 00 48 b8 00 00 00 00 00 fc ff df 48 8d bd 5f 02 00 00 48 89 fa 48 c1 ea 03 <0f> b6 04 02 48 89 fa 83 e2 07 38 d0 7f 08 84 c0 0f 85 c6 02 00 00
>> [...] RSP: 0018:ffffc90000f97788 EFLAGS: 00010207
>> [...] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
>> [...] RDX: 000000000000004b RSI: ffffffffc08cb843 RDI: 000000000000025f
>> [...] R10: fffffbfff5f718a0 R11: 0000000000000001 R12: dffffc0000000000
>> [...] R13: ffff888111085d40 R14: ffff888111085d40 R15: ffff888111080280
>> [...] FS:  00007f315f5db280(0000) GS:ffff8881f5200000(0000) knlGS:0000000000000000
>> [...] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [...] CR2: 000055bb25297170 CR3: 0000000110674000 CR4: 00000000000006f0
>> [...] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> [...] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> [...] Call Trace:
>> [...]  ? qlge_get_8012_flash_params+0x600/0x600 [qlge]
>> [...]  ? static_obj+0x8a/0xc0
>> [...]  ? lockdep_init_map_waits+0x26a/0x700
>> [...]  qlge_init_device+0x425/0x1000 [qlge]
>> [...]  ? debug_mutex_init+0x31/0x60
>> [...]  qlge_probe+0xfe/0x6c0 [qlge]
>> <snip>
>>
>> With qlge_get_8000_flash_params+0x377/0x6e0 corresponding to the following:
>>
>> 	if (qdev->flash.flash_params_8000.data_type1 == 2)
>> 		memcpy(mac_addr,
>> 		       qdev->flash.flash_params_8000.mac_addr1,
>> 		       qdev->ndev->addr_len); // <---- Here
>
>This is because qdev->ndev == 0.
>
>The reason is that before qlge_get_8000_flash_params() get called qdev is memset-ed inside qlge_init_device().
>
>static int qlge_init_device(struct pci_dev *pdev, struct qlge_adapter *qdev,
>				    int cards_found)
>	{
>	struct net_device *ndev = qdev->ndev;
>	int err = 0;
>
>	memset((void *)qdev, 0, sizeof(*qdev));
>
>	// ...
>
>	// qlge_get_8000_flash_params() get's called
>	err = qdev->nic_ops->get_flash(qdev);
>
>	// ...
>	}
>

Thank you for reporting this issue and providing the fix. I was a bit
confused after reading previous email because I dind't notice memset
and the address of ((struct net_device *) 0)->addr_len is 0x026f
which is outside the range reported by KASAN,

> [...] KASAN: null-ptr-deref in range [0x0000000000000258-0x000000000000025f]

--
Best regards,
Coiby
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel