linux-kernel - Re: [PATCH v3 2/2] net: macb: DMA-unmap full rx-buffer

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1394061949.2861.1.camel@deadeye.wl.decadent.org.uk>
Date:	Wed, 05 Mar 2014 23:25:49 +0000
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Soren Brinkmann <soren.brinkmann@...inx.com>
Cc:	Nicolas Ferre <nicolas.ferre@...el.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Vinod Koul <vinod.koul@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	"David S. Miller" <davem@...emloft.net>,
	Michal Simek <michal.simek@...inx.com>
Subject: Re: [PATCH v3 2/2] net: macb: DMA-unmap full rx-buffer

On Tue, 2014-03-04 at 08:46 -0800, Soren Brinkmann wrote:
> When allocating RX buffers a fixed size is used, while freeing is based
> on actually received bytes, resulting in the following kernel warning
> when CONFIG_DMA_API_DEBUG is enabled:
>  WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1051 check_unmap+0x258/0x894()
>  macb e000b000.ethernet: DMA-API: device driver frees DMA memory with different size [device address=0x000000002d170040] [map size=1536 bytes] [unmap size=60 bytes]
>  Modules linked in:
>  CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-rc3-xilinx-00220-g49f84081ce4f #65
>  [<c001516c>] (unwind_backtrace) from [<c0011df8>] (show_stack+0x10/0x14)
>  [<c0011df8>] (show_stack) from [<c03c775c>] (dump_stack+0x7c/0xc8)
>  [<c03c775c>] (dump_stack) from [<c00245cc>] (warn_slowpath_common+0x60/0x84)
>  [<c00245cc>] (warn_slowpath_common) from [<c0024670>] (warn_slowpath_fmt+0x2c/0x3c)
>  [<c0024670>] (warn_slowpath_fmt) from [<c0227d44>] (check_unmap+0x258/0x894)
>  [<c0227d44>] (check_unmap) from [<c0228588>] (debug_dma_unmap_page+0x64/0x70)
>  [<c0228588>] (debug_dma_unmap_page) from [<c02ab78c>] (gem_rx+0x118/0x170)
>  [<c02ab78c>] (gem_rx) from [<c02ac4d4>] (macb_poll+0x24/0x94)
>  [<c02ac4d4>] (macb_poll) from [<c031222c>] (net_rx_action+0x6c/0x188)
>  [<c031222c>] (net_rx_action) from [<c0028a28>] (__do_softirq+0x108/0x280)
>  [<c0028a28>] (__do_softirq) from [<c0028e8c>] (irq_exit+0x84/0xf8)
>  [<c0028e8c>] (irq_exit) from [<c000f360>] (handle_IRQ+0x68/0x8c)
>  [<c000f360>] (handle_IRQ) from [<c0008528>] (gic_handle_irq+0x3c/0x60)
>  [<c0008528>] (gic_handle_irq) from [<c0012904>] (__irq_svc+0x44/0x78)
>  Exception stack(0xc056df20 to 0xc056df68)
>  df20: 00000001 c0577430 00000000 c0577430 04ce8e0d 00000002 edfce238 00000000
>  df40: 04e20f78 00000002 c05981f4 00000000 00000008 c056df68 c0064008 c02d7658
>  df60: 20000013 ffffffff
>  [<c0012904>] (__irq_svc) from [<c02d7658>] (cpuidle_enter_state+0x54/0xf8)
>  [<c02d7658>] (cpuidle_enter_state) from [<c02d77dc>] (cpuidle_idle_call+0xe0/0x138)
>  [<c02d77dc>] (cpuidle_idle_call) from [<c000f660>] (arch_cpu_idle+0x8/0x3c)
>  [<c000f660>] (arch_cpu_idle) from [<c006bec4>] (cpu_startup_entry+0xbc/0x124)
>  [<c006bec4>] (cpu_startup_entry) from [<c053daec>] (start_kernel+0x350/0x3b0)
>  ---[ end trace d5fdc38641bd3a11 ]---
>  Mapped at:
>   [<c0227184>] debug_dma_map_page+0x48/0x11c
>   [<c02ab32c>] gem_rx_refill+0x154/0x1f8
>   [<c02ac7b4>] macb_open+0x270/0x3e0
>   [<c03152e0>] __dev_open+0x7c/0xfc
>   [<c031554c>] __dev_change_flags+0x8c/0x140
> 
> Fixing this by passing the same size which is passed during mapping the
> memory to the unmap function as well.
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>

Reviewed-by: Ben Hutchings <ben@...adent.org.uk>

> ---
> v3:
>  - no changes
> v2:
>  - no changes
> 
> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>
> ---
>  drivers/net/ethernet/cadence/macb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 1f03e2041e86..d0c38e01e99f 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -730,7 +730,7 @@ static int gem_rx(struct macb *bp, int budget)
>  		skb_put(skb, len);
>  		addr = MACB_BF(RX_WADDR, MACB_BFEXT(RX_WADDR, addr));
>  		dma_unmap_single(&bp->pdev->dev, addr,
> -				 len, DMA_FROM_DEVICE);
> +				 bp->rx_buffer_size, DMA_FROM_DEVICE);
>  
>  		skb->protocol = eth_type_trans(skb, bp->dev);
>  		skb_checksum_none_assert(skb);

-- 
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)