Skip to contentSkip to conversation

mm/cma: don't release CMA pages still in use

RFC PATCH Superseded Rik van Riel 2 participants 4 messages Aug 10 – Aug 11

Subsystems (2)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

RFC PATCH v2 1/1 mm/cma: don't release CMA pages still in use

When a driver calls dma_free_contiguous() before quiescing DMA, the page still has a reference from the device. put_page_testzero() there returns false, WARN fires, but the code proceeds to free_contig_frozen_range() putting a live page onto buddy and clearing the bitmap. Later cma_alloc() hands the same PFN to a new owner while the original holder still references it.

A concurrent put_page() that drops the last reference between the testzero loop and free_contig_frozen_range() can double-queue the page via page->lru, corrupting buddy lists.

Fix by freeing already-frozen pages in contiguous runs, while keeping still-referenced pages. When any live page exists, a future cma_alloc() on this address range will fail until whoever holds the extra references frees those pages.

This change should be safe because nothing can get reallocated while it is still in use.

Fixes 9bda131c6093 ("mm: cma: add cma_alloc_frozen{_compound}()")
Assisted-by Hermes:muse-spark-1.2 syzkaller

---
v2:
- don't play clever games with the CMA bitmap, the page allocator alone will prevent re-use of not-free-yet pages
- v1: https://lore.kernel.org/all/20260809210608.06b5ccb9@fangorn/

 mm/cma.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)
mm/cma.c +33 -6
index a13ce4999b39..cbf8dba8f077 100644
@@ -998,7 +998,6 @@ static void __cma_release_frozen(struct cma *cma, struct cma_memrange *cmr,
 
 	pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
 
-	free_contig_frozen_range(pfn, count);
 	cma_clear_bitmap(cma, cmr, pfn, count);
 	cma_sysfs_account_release_pages(cma, count);
 	trace_cma_release(cma->name, pfn, pages, count);
@@ -1018,18 +1017,45 @@ bool cma_release(struct cma *cma, const struct page *pages,
 		 unsigned long count)
 {
 	struct cma_memrange *cmr;
-	unsigned long ret = 0;
+	unsigned long skipped = 0;
 	unsigned long i, pfn;
+	unsigned long base_pfn;
+	unsigned long run_start = 0;
+	unsigned long run_len = 0;
 
 	cmr = find_cma_memrange(cma, pages, count);
 	if (!cmr)
 		return false;
 
-	pfn = page_to_pfn(pages);
-	for (i = 0; i < count; i++, pfn++)
-		ret += !put_page_testzero(pfn_to_page(pfn));
+	base_pfn = page_to_pfn(pages);
+	pfn = base_pfn;
+	for (i = 0; i < count; i++, pfn++) {
+		if (put_page_testzero(pfn_to_page(pfn))) {
+			/* Add it to the batch. */
+			if (run_len == 0)
+				run_start = pfn;
+			run_len++;
+		} else {
+			/*
+			 * This page is still in use! Free the freeable
+			 * pages encountered so far, but skip this page.
+			 */
+			if (run_len) {
+				free_contig_frozen_range(run_start, run_len);
+				run_len = 0;
+			}
+			skipped++;
+		}
+	}
+	if (run_len)
+		free_contig_frozen_range(run_start, run_len);
 
-	WARN(ret, "%lu pages are still in use!\n", ret);
+	/*
+	 * Some pages were still in use! This should not happen.
+	 * Subsequent cma_alloc() calls to the same range will fail
+	 * until whoever grabbed the extra refcounts frees the pages.
+	 */
+	WARN(skipped, "%lu pages are still in use!\n", skipped);
 
 	__cma_release_frozen(cma, cmr, pages, count);
 
@@ -1046,6 +1072,7 @@ bool cma_release_frozen(struct cma *cma, const struct page *pages,
 	if (!cmr)
 		return false;
 
+	free_contig_frozen_range(page_to_pfn(pages), count);
 	__cma_release_frozen(cma, cmr, pages, count);
 
 	return true;
Signature · 1 line
2.55.0
#1 lore raw
Quoted 6 lines · Rik van Riel

When a driver calls dma_free_contiguous() before quiescing DMA, the page still has a reference from the device. put_page_testzero() there returns false, WARN fires, but the code proceeds to free_contig_frozen_range() putting a live page onto buddy and clearing the bitmap. Later cma_alloc() hands the same PFN to a new owner while the original holder still references it.

Hm, it rather sounds like it's really the drivers job to quiescing DMA. Are you aware of an in-tree driver that can trigger this, and if so, how?

Freeing memory through CMA, while the memory is still in use is problematic for CMA as you discovered :)

Quoted 77 lines

A concurrent put_page() that drops the last reference between the testzero loop and free_contig_frozen_range() can double-queue the page via page->lru, corrupting buddy lists.

Fix by freeing already-frozen pages in contiguous runs, while keeping still-referenced pages. When any live page exists, a future cma_alloc() on this address range will fail until whoever holds the extra references frees those pages.

This change should be safe because nothing can get reallocated while it is still in use.

Fixes 9bda131c6093 ("mm: cma: add cma_alloc_frozen{_compound}()")
Assisted-by Hermes:muse-spark-1.2 syzkaller

---
v2:
- don't play clever games with the CMA bitmap, the page allocator alone will prevent re-use of not-free-yet pages
- v1: https://lore.kernel.org/all/20260809210608.06b5ccb9@fangorn/

 mm/cma.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)
mm/cma.c +24 -5
index a13ce4999b39..cbf8dba8f077 100644
@@ -998,7 +998,6 @@ static void __cma_release_frozen(struct cma *cma, struct cma_memrange *cmr,
 
 	pr_debug("%s(page %p, count %lu)\n", __func__, (void *)pages, count);
 
-	free_contig_frozen_range(pfn, count);
 	cma_clear_bitmap(cma, cmr, pfn, count);
 	cma_sysfs_account_release_pages(cma, count);
 	trace_cma_release(cma->name, pfn, pages, count);
@@ -1018,18 +1017,45 @@ bool cma_release(struct cma *cma, const struct page *pages,
 		 unsigned long count)
 {
 	struct cma_memrange *cmr;
-	unsigned long ret = 0;
+	unsigned long skipped = 0;
 	unsigned long i, pfn;
+	unsigned long base_pfn;
+	unsigned long run_start = 0;
+	unsigned long run_len = 0;
 
 	cmr = find_cma_memrange(cma, pages, count);
 	if (!cmr)
 		return false;
 
-	pfn = page_to_pfn(pages);
-	for (i = 0; i < count; i++, pfn++)
-		ret += !put_page_testzero(pfn_to_page(pfn));
+	base_pfn = page_to_pfn(pages);
+	pfn = base_pfn;
+	for (i = 0; i < count; i++, pfn++) {
+		if (put_page_testzero(pfn_to_page(pfn))) {
+			/* Add it to the batch. */
+			if (run_len == 0)
+				run_start = pfn;
+			run_len++;
+		} else {
+			/*
+			 * This page is still in use! Free the freeable
+			 * pages encountered so far, but skip this page.
+			 */
+			if (run_len) {
+				free_contig_frozen_range(run_start, run_len);
+				run_len = 0;
+			}
+			skipped++;
+		}
+	}

mm/cma.c bool cma_release(struct cma *cma, const struct page *pages, · line 1058

Quoted 10 lines
mm/cma.c +8 -1
+	if (run_len)
+		free_contig_frozen_range(run_start, run_len);
 
-	WARN(ret, "%lu pages are still in use!\n", ret);
+	/*
+	 * Some pages were still in use! This should not happen.
+	 * Subsequent cma_alloc() calls to the same range will fail
+	 * until whoever grabbed the extra refcounts frees the pages.
+	 */
+	WARN(skipped, "%lu pages are still in use!\n", skipped);

We still issue a WARN, which itself is problematic. See "Do not WARN lightly" in Documentation/process/coding-style.rst.

So it's either

(a) This scenario is valid to be triggered. In that case, WARN is not appropriate.

(b) This scenario is not valid to be triggered. In that case, this is not a fix.

Signature · 3 lines
Cheers,

David
#2 lore raw
Quoted 11 lines · David Hildenbrand (Arm)
Quoted 9 lines · Rik van Riel

When a driver calls dma_free_contiguous() before quiescing DMA, the page still has a reference from the device. put_page_testzero() there
returns false, WARN fires, but the code proceeds to free_contig_frozen_range() putting a live page onto buddy and clearing
the bitmap. Later cma_alloc() hands the same PFN to a new owner while
the original holder still references it.

Quoted 7 lines

Hm, it rather sounds like it's really the drivers job to quiescing DMA. Are you
aware of an in-tree driver that can trigger this, and if so, how?

Freeing memory through CMA, while the memory is still in use is problematic for
CMA as you discovered :)

I agree the driver should not be doing that.

I do not know of any driver that currently does it, but it has led to CVE-2025-37837 in the past, and there appear to have been other cases of this issue happening.

Quoted 7 lines

mm/cma.c bool cma_release(struct cma *cma, const struct page *pages, · line 1058

Quoted 1 line
mm/cma.c +1 -0
+	WARN(skipped, "%lu pages are still in use!\n", skipped);

We still issue a WARN, which itself is problematic. See "Do not WARN lightly" in
Documentation/process/coding-style.rst.

Quoted 7 lines

So it's either

(a) This scenario is valid to be triggered. In that case, WARN is not appropriate.

(b) This scenario is not valid to be triggered. In that case, this is not a fix.

If a driver does it with the current code, it can cause free list corruption, with potential for temporary data exposure, until the system crashes.

If we get rid of the free list corruption, it can continue to be a WARN, since it will no longer kill the system.

If we continue to corrupt the free list when this happens, it should probably be a panic?

A simpler fix may be to not free any of the memory, or the CMA address space, if there are still pages in use at cma_release() time.

Signature · 1 line
All Rights Reversed.
#3 lore raw
Quoted 49 lines · Rik van Riel
Quoted 11 lines · David Hildenbrand (Arm)
Quoted 9 lines · Rik van Riel

When a driver calls dma_free_contiguous() before quiescing DMA, the page still has a reference from the device. put_page_testzero() there
returns false, WARN fires, but the code proceeds to free_contig_frozen_range() putting a live page onto buddy and clearing
the bitmap. Later cma_alloc() hands the same PFN to a new owner while
the original holder still references it.

Quoted 7 lines

Hm, it rather sounds like it's really the drivers job to quiescing DMA. Are you
aware of an in-tree driver that can trigger this, and if so, how?

Freeing memory through CMA, while the memory is still in use is problematic for
CMA as you discovered :)

I agree the driver should not be doing that.

I do not know of any driver that currently does it, but it has led to CVE-2025-37837 in the past, and there appear to have been other cases of this issue happening.

Quoted 7 lines

mm/cma.c bool cma_release(struct cma *cma, const struct page *pages, · line 1058

Quoted 1 line
mm/cma.c +1 -0
+	WARN(skipped, "%lu pages are still in use!\n", skipped);

We still issue a WARN, which itself is problematic. See "Do not WARN lightly" in
Documentation/process/coding-style.rst.

Quoted 7 lines

So it's either

(a) This scenario is valid to be triggered. In that case, WARN is not appropriate.

(b) This scenario is not valid to be triggered. In that case, this is not a fix.

If a driver does it with the current code, it can cause free list corruption, with potential for temporary data exposure, until the system crashes.

If we get rid of the free list corruption, it can continue to be a WARN, since it will no longer kill the system.

Quoted 6 lines

If we continue to corrupt the free list when this happens, it should probably be a panic?

A simpler fix may be to not free any of the memory, or the CMA address space, if there are still pages in use at cma_release() time.

Yes, keep it as simple as possible and complain.

This is really something that must not happen, and the CVE in that above case is not this code's fault but the driver's fault.

Signature · 3 lines
Cheers,

David