netdev - Re: [PATCH] igb: Fix TX map failure path

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] [day] [month] [year] [list]

Date:   Thu, 19 Oct 2017 12:58:30 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Jean-Philippe Brucker <jean-philippe.brucker@....com>
Cc:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        intel-wired-lan <intel-wired-lan@...ts.osuosl.org>,
        Netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] igb: Fix TX map failure path

On Thu, Oct 19, 2017 at 12:07 PM, Jean-Philippe Brucker
<jean-philippe.brucker@....com> wrote:
> When the driver cannot map a TX buffer, instead of rolling back
> gracefully and retrying later, we currently get a panic:
>
> [  159.885994] igb 0000:00:00.0: TX DMA map failed
> [  159.886588] Unable to handle kernel paging request at virtual address ffff00000a08c7a8
>                ...
> [  159.897031] PC is at igb_xmit_frame_ring+0x9c8/0xcb8
>
> Fix the erroneous test that leads to this situation.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@....com>
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index fd4a46b03cc8..ea69af267d63 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -5326,7 +5326,7 @@ static int igb_tx_map(struct igb_ring *tx_ring,
>                                        DMA_TO_DEVICE);
>                 dma_unmap_len_set(tx_buffer, len, 0);
>
> -               if (i--)
> +               if (i-- == 0)
>                         i += tx_ring->count;
>                 tx_buffer = &tx_ring->tx_buffer_info[i];
>         }

The fix looks fine to me, though I will probably want to go back and
update the code to simplify this since I realize part of the issue is
that there is a bunch of unneeded complexity I introduced here. It
looks like we have the same problem in ixgbe that needs to be
addressed as well.

Fixes: 7cc6fd4c60f2 ("igb: Don't bother clearing Tx buffer_info in
igb_clean_tx_ring")
Acked-by: Alexander Duyck <alexander.h.duyck@...el.com>