Re: [PATCH] common/mlx5: fix overlapping memory ranges

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 3 Aug 2026 12:18:50 +0300
Raslan Darawsheh <rasland@nvidia.com> wrote:

> Hi,
> 
> 
> On 21/07/2026 3:13 PM, Viacheslav Ovsiienko wrote:
> > The mlx5 driver requires special objects named Memory Regions
> > (MR) to perform DMA operations with network data. The memory
> > pool(s) is used to provide memory and to cover pool addresses
> > the mlx5 PMD -pre-creates the appropriate MRs on Rx queue creation.
> > 
> > The pool memory can be non-contigous and split into segments.
> > The PMD created MRs on the page alignment segment boundaries
> > and it could cause the overlapping MRs (in case if the end
> > address of one segmend is aligned to ceiling and the next
> > segment start address is aligned to the floor).
> > 
> > The MRs overlapping could cause the wrong MR fetching from the
> > cache for the mbufs in the overlapping area if the starting
> > mbuf address falls into overlapped area and raise the
> > hardware memory protection exception.
> > 
> > Fixes: 690b2a88c2f7 ("common/mlx5: add mempool registration facilities")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> > Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>  
> 
> Patch applied to next-net-mlx,
> 
> Kindest regards
> Raslan Darawsheh
> 

More detailed AI review found problems with this patch.

 Finding: f718141d6c "common/mlx5: fix overlapping memory ranges" — incomplete fix

  The change from != to < correctly merges the page-alignment overlap case
  (chunks[i-1].end > chunks[i].start). But the merge body still propagates the
  previous chunk's end rather than the maximum end seen so far:

  for (i = 1; i < chunks_n; i++)
      if (chunks[i - 1].end < chunks[i].start) {
          chunks[contig_n - 1].end = chunks[i - 1].end;   /* mlx5_common_mr.c:1507 
  */

  That is only safe if ends are monotonically non-decreasing after the sort.
  mlx5_range_compare_start (mlx5_common_mr.c:1351) compares start only, and qsort
  is not stable — so two ranges with equal starts but different ends can sort in
  either order. Equal starts are reachable on the regular-chunk path:
  mlx5_range_from_mempool_chunk (:1372) floors the start to a page, so two raw
  mempool chunks in the same page both yield start = P, while their ends ceil to
  different pages.

  Concretely, for raw chunks producing [P, P+2pg] and [P, P+pg] sorted in that
  order, the merge yields a final end of P+pg — the last pg of registered memory is
  dropped, which is the same class of MR-coverage bug the commit sets out to fix,
  just in the opposite direction (under-coverage rather than overlap).

  The extmem path is unaffected: mlx5_mempool_get_extmem_cb (:1447) emits uniform
  single-page segments, so equal starts imply equal ends there.

  Suggested fix — track the running maximum:

  chunks[contig_n - 1].end = RTE_MAX(chunks[contig_n - 1].end, chunks[i - 1].end);

  applied at both the in-loop assignment and the post-loop "extend the last chunk"
  line. Alternatively, extend the comparator to break ties on descending end, which
  restores the monotonicity the current code assumes.