Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] The order-0 allocation fallback loop causes a heap buffer overflow and memory leaks when migrating a partial THP or when some subpages lack the MIGRATE_PFN_MIGRATE flag. --
--- Patch [1]: [PATCH] drm/pagemap: Fix folio allocation fallback and use-after-put --- commit 0e0b48c51914bc3498ff387aa623528489adafed Author: Matthew Brost <[email protected]> drm/pagemap: Fix folio allocation fallback and use-after-put This commit fixes a missing __GFP_NOWARN in higher-order folio allocations and implements a fallback to order-0 allocations when THP allocation fails. It also fixes a use-after-free issue where folio_order() was called after dropping the page reference. Cc: [email protected] > diff --git a/drivers/gpu/drm/drm_pagemap.c b/drivers/gpu/drm/drm_pagemap.c > index 15c78eca180b7..407c7546e168a 100644 > --- a/drivers/gpu/drm/drm_pagemap.c > +++ b/drivers/gpu/drm/drm_pagemap.c [ ... ] > @@ -886,11 +886,34 @@ static int drm_pagemap_migrate_populate_ram_pfn(struct > vm_area_struct *vas, > > order = folio_order(page_folio(src_page)); > > - /* TODO: Support fallback to single pages if THP allocation > fails */ > if (vas) > - folio = vma_alloc_folio(GFP_HIGHUSER, order, vas, addr); > + folio = vma_alloc_folio(GFP_HIGHUSER | __GFP_NOWARN, > order, vas, addr); > else > - folio = folio_alloc(GFP_HIGHUSER, order); > + folio = folio_alloc(GFP_HIGHUSER | __GFP_NOWARN, order); > + > + if (!folio && order) { > + /* > + * Higher-order allocation failed, fall back to > + * order-0 allocations for the entire range covered > + * by the original higher-order allocation, without > + * setting MIGRATE_PFN_COMPOUND, until we move past > + * that range. > + */ > + unsigned long nr = NR_PAGES(order); > + unsigned long j; > + > + for (j = 0; j < nr; j++, i++, addr += PAGE_SIZE) { > + folio = vas ? > + vma_alloc_folio(GFP_HIGHUSER, 0, vas, > addr) : > + folio_alloc(GFP_HIGHUSER, 0); > + if (!folio) > + goto free_pages; > + > + page = folio_page(folio, 0); > + mpfn[i] = migrate_pfn(page_to_pfn(page)); [Severity: Critical] Can this fallback loop write past the end of the mpfn array? If the migration request only covers a partial THP, or if we are near the end of the npages limit, nr could be larger than the remaining entries (npages - i). Since this loop unconditionally increments i and writes to mpfn[i] nr times, it appears it could cause a heap buffer overflow. Additionally, this loop doesn't check if the subpages have the MIGRATE_PFN_MIGRATE flag set in src_mpfn. Can this cause memory leaks if we allocate and assign order-0 folios for subpages that weren't actually requested for migration and thus might not be cleaned up later? > + } > + continue; > + } > > if (!folio) > goto free_pages; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
