Re: [PATCH iwl-net v2 2/2] e1000e: fix ps_pages DMA map error sentinel

["Kadosh, MoriyaX" <moriyax.kadosh@intel.com>]

On 05/10/2026 21:11, Ruinskiy, Dima wrote:
> 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>
Tested-by: Moriya Kadosh <moriyax.kadosh@intel.com>