netdev - Re: [Intel-wired-lan] [PATCH] e1000e: allocate ring descriptors with dma_zalloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3a4dceae-de33-4270-bbed-5aaf8c82a7bd@intel.com>
Date:   Sun, 4 Feb 2018 10:09:07 +0200
From:   "Neftin, Sasha" <sasha.neftin@...el.com>
To:     Pierre-Yves Kerbrat <pkerbrat@...ray.eu>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        intel-wired-lan@...ts.osuosl.org
Cc:     netdev@...r.kernel.org, Marius Gligor <mgligor@...ray.eu>
Subject: Re: [Intel-wired-lan] [PATCH] e1000e: allocate ring descriptors with
 dma_zalloc_coherent

On 1/26/2018 12:24, Pierre-Yves Kerbrat wrote:
> Descriptor rings were not initialized at zero when allocated
> When area contained garbage data, it caused skb_over_panic in
> e1000_clean_rx_irq (if data had E1000_RXD_STAT_DD bit set)
> 
> This patch makes use of dma_zalloc_coherent to make sure the
> ring is memset at 0 to prevent the area from containing garbage.
> 
> Following is the signature of the panic:
> IODDR0@0.0: skbuff: skb_over_panic: text:80407b20 len:64010 put:64010 head:ab46d800 data:ab46d842 tail:0xab47d24c end:0xab46df40 dev:eth0
> IODDR0@0.0: BUG: failure at net/core/skbuff.c:105/skb_panic()!
> IODDR0@0.0: Kernel panic - not syncing: BUG!
> IODDR0@0.0:
> IODDR0@0.0: Process swapper/0 (pid: 0, threadinfo=81728000, task=8173cc00 ,cpu: 0)
> IODDR0@0.0: SP = <815a1c0c>
> IODDR0@0.0: Stack:      00000001
> IODDR0@0.0: b2d89800 815e33ac
> IODDR0@0.0: ea73c040 00000001
> IODDR0@0.0: 60040003 0000fa0a
> IODDR0@0.0: 00000002
> IODDR0@0.0:
> IODDR0@0.0: 804540c0 815a1c70
> IODDR0@0.0: b2744000 602ac070
> IODDR0@0.0: 815a1c44 b2d89800
> IODDR0@0.0: 8173cc00 815a1c08
> IODDR0@0.0:
> IODDR0@0.0:     00000006
> IODDR0@0.0: 815a1b50 00000000
> IODDR0@0.0: 80079434 00000001
> IODDR0@0.0: ab46df40 b2744000
> IODDR0@0.0: b2d89800
> IODDR0@0.0:
> IODDR0@0.0: 0000fa0a 8045745c
> IODDR0@0.0: 815a1c88 0000fa0a
> IODDR0@0.0: 80407b20 b2789f80
> IODDR0@0.0: 00000005 80407b20
> IODDR0@0.0:
> IODDR0@0.0:
> IODDR0@0.0: Call Trace:
> IODDR0@0.0: [<804540bc>] skb_panic+0xa4/0xa8
> IODDR0@0.0: [<80079430>] console_unlock+0x2f8/0x6d0
> IODDR0@0.0: [<80457458>] skb_put+0xa0/0xc0
> IODDR0@0.0: [<80407b1c>] e1000_clean_rx_irq+0x2dc/0x3e8
> IODDR0@0.0: [<80407b1c>] e1000_clean_rx_irq+0x2dc/0x3e8
> IODDR0@0.0: [<804079c8>] e1000_clean_rx_irq+0x188/0x3e8
> IODDR0@0.0: [<80407b1c>] e1000_clean_rx_irq+0x2dc/0x3e8
> IODDR0@0.0: [<80468b48>] __dev_kfree_skb_any+0x88/0xa8
> IODDR0@0.0: [<804101ac>] e1000e_poll+0x94/0x288
> IODDR0@0.0: [<8046e9d4>] net_rx_action+0x19c/0x4e8
> IODDR0@0.0:   ...
> IODDR0@0.0: Maximum depth to print reached. Use kstack=<maximum_depth_to_print> To specify a custom value (where 0 means to display the full backtrace)
> IODDR0@0.0: ---[ end Kernel panic - not syncing: BUG!
> 
> Signed-off-by: Pierre-Yves Kerbrat <pkerbrat@...ray.eu>
> Signed-off-by: Marius Gligor <mgligor@...ray.eu>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 1298b69..26121ed 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -2333,7 +2333,7 @@ static int e1000_alloc_ring_dma(struct e1000_adapter *adapter,
>   {
>   	struct pci_dev *pdev = adapter->pdev;
>   
> -	ring->desc = dma_alloc_coherent(&pdev->dev, ring->size, &ring->dma,
> +	ring->desc = dma_zalloc_coherent(&pdev->dev, ring->size, &ring->dma,
>   					GFP_KERNEL);
>   	if (!ring->desc)
>   		return -ENOMEM;
> 
Looks good. Prefer get another opinion from somebody else.