Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Ruinskiy, Dima" <dima.ruinskiy@intel.com>
To: Matt Vollrath <tactii@gmail.com>, <intel-wired-lan@lists.osuosl.org>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>, <netdev@vger.kernel.org>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH iwl-net v2 2/2] e1000e: fix ps_pages DMA map error sentinel
Date: Mon, 5 Oct 2026 21:11:18 +0300	[thread overview]
Message-ID: <86add723-6e77-433b-83db-d186d98140ce@intel.com> (raw)
In-Reply-To: <20260913171555.89537-3-tactii@gmail.com>

On 13/09/2026 20:15, Matt Vollrath wrote:
> While allocating packet-split buffer pages, a failed DMA mapping would
> leave DMA_MAPPING_ERROR in the ps_page->dma field.
> 
> This would have two consequences:
> * The next attempt to allocate that buffer would write DMA_MAPPING_ERROR
>    to h/w if all pages are allocated. If the h/w uses that buffer and is
>    handling a frame large enough to touch the affected page, it would
>    cause a DMA fault and be dropped. The driver would then call
>    dma_unmap_page() on DMA_MAPPING_ERROR and unknowingly send the
>    uninitialized page up the stack as part of the frame payload.
> * On ring teardown, dma_unmap_page() would be called on
>    DMA_MAPPING_ERROR.
> 
> This condition is only reachable when MTU > 1500 and PAGE_SIZE <= 16K.
> 
> Fix this by setting ps_page->dma = 0 upon mapping failure and separately
> testing ->page and ->dma during allocation and teardown.
> 
> The rewrite of the ps_pages section of e1000_clean_rx_ring was necessary
> to recognize the case of an allocated page without a valid DMA mapping.
> It also fixes a separate bug which would potentially leak pages on ring
> teardown. The cleaner stops cleaning pages when h/w reported that it did
> not write to a page in the sequence, leaving the following pages
> allocated and mapped. The teardown would then break early and leak the
> unused mapped pages. If the ring is re-constructed by down/up with
> similar configuration, it would reclaim those lost pages. This would
> only affect configurations with rx_ps_pages >= 2 (MTU > PAGE_SIZE) and
> the same condition of MTU > 1500 and PAGE_SIZE <= 16K.
> 
> Signed-off-by: Matt Vollrath <tactii@gmail.com>
> Assisted-by: Claude:claude-5-fable
> Fixes: bc7f75fa9788 ("[E1000E]: New pci-express e1000 driver (currently for ICH9 devices only)")
> Cc: stable@vger.kernel.org
> ---
> v2:
> * Reword last paragraph of description.
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 26f45ee8c7e7..063fc8cd2673 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -759,12 +759,15 @@ static void e1000_alloc_rx_buffers_ps(struct e1000_ring *rx_ring,
>   					adapter->alloc_rx_buff_failed++;
>   					goto no_buffers;
>   				}
> +			}
> +			if (!ps_page->dma) {
>   				ps_page->dma = dma_map_page(&pdev->dev,
>   							    ps_page->page,
>   							    0, PAGE_SIZE,
>   							    DMA_FROM_DEVICE);
>   				if (dma_mapping_error(&pdev->dev,
>   						      ps_page->dma)) {
> +					ps_page->dma = 0;
>   					dev_err(&adapter->pdev->dev,
>   						"Rx DMA page map failed\n");
>   					adapter->rx_dma_failed++;
> @@ -1722,13 +1725,15 @@ static void e1000_clean_rx_ring(struct e1000_ring *rx_ring)
>   
>   		for (j = 0; j < PS_PAGE_BUFFERS; j++) {
>   			ps_page = &buffer_info->ps_pages[j];
> -			if (!ps_page->page)
> -				break;
> -			dma_unmap_page(&pdev->dev, ps_page->dma, PAGE_SIZE,
> -				       DMA_FROM_DEVICE);
> -			ps_page->dma = 0;
> -			put_page(ps_page->page);
> -			ps_page->page = NULL;
> +			if (ps_page->dma) {
> +				dma_unmap_page(&pdev->dev, ps_page->dma,
> +					       PAGE_SIZE, DMA_FROM_DEVICE);
> +				ps_page->dma = 0;
> +			}
> +			if (ps_page->page) {
> +				put_page(ps_page->page);
> +				ps_page->page = NULL;
> +			}
>   		}
>   	}
>   
Reviewed-by: Dima Ruinskiy <dima.ruinskiy@intel.com>

  reply	other threads:[~2026-10-05 18:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-13 17:15 [PATCH iwl-net v2 0/2] e1000e: fix Rx DMA error handling Matt Vollrath
2026-09-13 17:15 ` [PATCH iwl-net v2 1/2] e1000e: fix Rx skb DMA map error sentinel Matt Vollrath
2026-10-05 18:11   ` Ruinskiy, Dima
2026-10-05 19:47     ` Kadosh, MoriyaX
2026-09-13 17:15 ` [PATCH iwl-net v2 2/2] e1000e: fix ps_pages " Matt Vollrath
2026-10-05 18:11   ` Ruinskiy, Dima [this message]
2026-10-05 19:46     ` Kadosh, MoriyaX
2026-09-16 12:16 ` [PATCH iwl-net v2 0/2] e1000e: fix Rx DMA error handling Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=86add723-6e77-433b-83db-d186d98140ce@intel.com \
    --to=dima.ruinskiy@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=stable@vger.kernel.org \
    --cc=tactii@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox