On Wed, 2015-02-25 at 12:26 +0100, Sabrina Dubroca wrote:
> There is a race condition between e1000_change_mtu's cleanups and
> netpoll, when we change the MTU across jumbo size:
>
> Changing MTU frees all the rx buffers:
> e1000_change_mtu -> e1000_down -> e1000_clean_all_rx_rings ->
> e1000_clean_rx_ring
>
> Then, close to the end of e1000_change_mtu:
> pr_info -> ... -> netpoll_poll_dev -> e1000_clean ->
> e1000_clean_rx_irq -> e1000_alloc_rx_buffers ->
> e1000_alloc_frag
>
> And when we come back to do the rest of the MTU change:
> e1000_up -> e1000_configure -> e1000_configure_rx ->
> e1000_alloc_jumbo_rx_buffers
>
> alloc_jumbo finds the buffers already != NULL, since data (shared with
> page in e1000_rx_buffer->rxbuf) has been re-alloc'd, but it's garbage,
> or at least not what is expected when in jumbo state.
>
> This results in an unusable adapter (packets don't get through), and a
> NULL pointer dereference on the next call to e1000_clean_rx_ring
> (other mtu change, link down, shutdown):
>
> BUG: unable to handle kernel NULL pointer dereference at
> (null)
> IP: [<ffffffff81194d6e>] put_compound_page+0x7e/0x330
>
> [...]
>
> Call Trace:
> [<ffffffff81195445>] put_page+0x55/0x60
> [<ffffffff815d9f44>] e1000_clean_rx_ring+0x134/0x200
> [<ffffffff815da055>] e1000_clean_all_rx_rings+0x45/0x60
> [<ffffffff815df5e0>] e1000_down+0x1c0/0x1d0
> [<ffffffff811e2260>] ? deactivate_slab+0x7f0/0x840
> [<ffffffff815e21bc>] e1000_change_mtu+0xdc/0x170
> [<ffffffff81647050>] dev_set_mtu+0xa0/0x140
> [<ffffffff81664218>] do_setlink+0x218/0xac0
> [<ffffffff814459e9>] ? nla_parse+0xb9/0x120
> [<ffffffff816652d0>] rtnl_newlink+0x6d0/0x890
> [<ffffffff8104f000>] ? kvm_clock_read+0x20/0x40
> [<ffffffff810a2068>] ? sched_clock_cpu+0xa8/0x100
> [<ffffffff81663802>] rtnetlink_rcv_msg+0x92/0x260
>
> By setting the allocator to a dummy version, netpoll can't mess up our
> rx buffers. The allocator is set back to a sane value in
> e1000_configure_rx.
>
> Fixes: edbbb3ca1077 ("e1000: implement jumbo receive with partial
> descriptors")
> Signed-off-by: Sabrina Dubroca <s...@queasysnail.net>
>
> ---
>
> v2: Eric Dumazet suggested to remove the forward declaration of
> e1000_alloc_dummy_rx_buffers
> v3: cleanup whitespace from v1
>
> drivers/net/ethernet/intel/e1000/e1000_main.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)I have update the patch in my queue, to this latest version. Thanks
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________ E1000-devel mailing list E1000-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
