Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split()
@ 2026-08-05  8:42 dayou5941
  2026-08-05  9:32 ` Lorenzo Stoakes (ARM)
  2026-08-19 19:14 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: dayou5941 @ 2026-08-05  8:42 UTC (permalink / raw)
  To: akpm, david, ljs; +Cc: ziy, linux-mm, Li Youhong, Sashiko, stable

From: Li Youhong <liyouhong@kylinos.cn>

min_order_for_split() accesses folio->mapping without proper
synchronization. While the compiler typically caches the value
in a register making a NULL deref unlikely in practice, the
real issue is that the callers in memory-failure.c do not hold
the folio lock at the time of the call:

- memory_failure() explicitly drops the folio lock before calling
  min_order_for_split().
- soft_offline_in_use_page() has not yet acquired the folio lock
  when calling min_order_for_split().

This means the value of folio->mapping may be modified by a
truncate or invalidate operation while min_order_for_split() is
executing, leading to a torn read or use of a stale mapping value.

Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260803060001.800638-1-dayou5941@163.com
Cc: stable@vger.kernel.org
Signed-off-by: Li Youhong <liyouhong@kylinos.cn>
---
v2:
- Dropped the approach of caching folio->mapping inside min_order_for_split() in favor of adding the folio lock at the callers.
- Added VM_WARN_ON_ONCE_FOLIO() in min_order_for_split().
- Updated the commit message to clarify that the real issue is the callers not holding the folio lock, rather than a TOCTOU race.
  v1: https://lore.kernel.org/all/20260804035828.2684059-1-dayou5941@163.com/

v3:
- Fix author name and Signed-off-by format as requested by Greg.
  v2: https://lore.kernel.org/all/20260805072625.2437636-1-dayou5941@163.com/
---
 mm/huge_memory.c    |  2 ++
 mm/memory-failure.c | 12 ++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 58cabe6af33d..e3f16dadc1d4 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -4300,6 +4300,8 @@ int folio_split(struct folio *folio, unsigned int new_order,
  */
 unsigned int min_order_for_split(struct folio *folio)
 {
+	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
+
 	if (folio_test_anon(folio))
 		return 0;
 
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 3b1e6946821b..7391524b5046 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -2437,12 +2437,17 @@ int memory_failure(unsigned long pfn, int flags)
 		res = -EOPNOTSUPP;
 		goto unlock_mutex;
 	}
+
 	folio_unlock(folio);
 
 	if (folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
+		const int new_order;
 		int err;
 
+		folio_lock(folio);
+		new_order = min_order_for_split(folio);
+		folio_unlock(folio);
+
 		/*
 		 * The flag must be set after the refcount is bumped
 		 * otherwise it may race with THP split.
@@ -2796,8 +2801,11 @@ static int soft_offline_in_use_page(struct page *page)
 	};
 
 	if (!huge && folio_test_large(folio)) {
-		const int new_order = min_order_for_split(folio);
+		const int new_order;
 
+		folio_lock(folio);
+		new_order = min_order_for_split(folio);
+		folio_unlock(folio);
 		/*
 		 * If new_order (target split order) is not 0, do not split the
 		 * folio at all to retain the still accessible large folio.
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split()
  2026-08-05  8:42 [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split() dayou5941
@ 2026-08-05  9:32 ` Lorenzo Stoakes (ARM)
  2026-08-19 19:14 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Lorenzo Stoakes (ARM) @ 2026-08-05  9:32 UTC (permalink / raw)
  To: dayou5941; +Cc: akpm, david, ziy, linux-mm, Li Youhong, Sashiko, stable

Please stop sending series so quickly :)

Leave at least a day between new revisions.

There's review on the v2 that is outstanding.

So engage in discussions there and wait until tomorrow before sending v4.

Thanks!

On Wed, Aug 05, 2026 at 04:42:24PM +0800, dayou5941@163.com wrote:
> From: Li Youhong <liyouhong@kylinos.cn>
>
> min_order_for_split() accesses folio->mapping without proper
> synchronization. While the compiler typically caches the value
> in a register making a NULL deref unlikely in practice, the
> real issue is that the callers in memory-failure.c do not hold
> the folio lock at the time of the call:
>
> - memory_failure() explicitly drops the folio lock before calling
>   min_order_for_split().
> - soft_offline_in_use_page() has not yet acquired the folio lock
>   when calling min_order_for_split().
>
> This means the value of folio->mapping may be modified by a
> truncate or invalidate operation while min_order_for_split() is
> executing, leading to a torn read or use of a stale mapping value.
>
> Fixes: 689b8986776c ("mm/memory-failure: improve large block size folio handling")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260803060001.800638-1-dayou5941@163.com
> Cc: stable@vger.kernel.org
> Signed-off-by: Li Youhong <liyouhong@kylinos.cn>
> ---
> v2:
> - Dropped the approach of caching folio->mapping inside min_order_for_split() in favor of adding the folio lock at the callers.
> - Added VM_WARN_ON_ONCE_FOLIO() in min_order_for_split().
> - Updated the commit message to clarify that the real issue is the callers not holding the folio lock, rather than a TOCTOU race.
>   v1: https://lore.kernel.org/all/20260804035828.2684059-1-dayou5941@163.com/
>
> v3:
> - Fix author name and Signed-off-by format as requested by Greg.
>   v2: https://lore.kernel.org/all/20260805072625.2437636-1-dayou5941@163.com/
> ---
>  mm/huge_memory.c    |  2 ++
>  mm/memory-failure.c | 12 ++++++++++--
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 58cabe6af33d..e3f16dadc1d4 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4300,6 +4300,8 @@ int folio_split(struct folio *folio, unsigned int new_order,
>   */
>  unsigned int min_order_for_split(struct folio *folio)
>  {
> +	VM_WARN_ON_ONCE_FOLIO(!folio_test_locked(folio), folio);
> +
>  	if (folio_test_anon(folio))
>  		return 0;
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 3b1e6946821b..7391524b5046 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2437,12 +2437,17 @@ int memory_failure(unsigned long pfn, int flags)
>  		res = -EOPNOTSUPP;
>  		goto unlock_mutex;
>  	}
> +
>  	folio_unlock(folio);
>
>  	if (folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> +		const int new_order;
>  		int err;
>
> +		folio_lock(folio);
> +		new_order = min_order_for_split(folio);
> +		folio_unlock(folio);
> +
>  		/*
>  		 * The flag must be set after the refcount is bumped
>  		 * otherwise it may race with THP split.
> @@ -2796,8 +2801,11 @@ static int soft_offline_in_use_page(struct page *page)
>  	};
>
>  	if (!huge && folio_test_large(folio)) {
> -		const int new_order = min_order_for_split(folio);
> +		const int new_order;
>
> +		folio_lock(folio);
> +		new_order = min_order_for_split(folio);
> +		folio_unlock(folio);
>  		/*
>  		 * If new_order (target split order) is not 0, do not split the
>  		 * folio at all to retain the still accessible large folio.
> --
> 2.25.1
>

--
Cheers, Lorenzo


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split()
  2026-08-05  8:42 [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split() dayou5941
  2026-08-05  9:32 ` Lorenzo Stoakes (ARM)
@ 2026-08-19 19:14 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2026-08-19 19:14 UTC (permalink / raw)
  To: dayou5941, akpm, david, ljs
  Cc: oe-kbuild-all, ziy, linux-mm, Li Youhong, Sashiko, stable

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/dayou5941-163-com/mm-memory-failure-fix-concurrent-access-issue-in-min_order_for_split/20260805-164224
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20260805084224.2597547-1-dayou5941%40163.com
patch subject: [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split()
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20260820/202608200310.aIH2BEKj-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260820/202608200310.aIH2BEKj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608200310.aIH2BEKj-lkp@intel.com/

All errors (new ones prefixed by >>):

   mm/memory-failure.c: In function 'memory_failure':
>> mm/memory-failure.c:2517:27: error: assignment of read-only variable 'new_order'
    2517 |                 new_order = min_order_for_split(folio);
         |                           ^
   mm/memory-failure.c: In function 'soft_offline_in_use_page':
   mm/memory-failure.c:2873:27: error: assignment of read-only variable 'new_order'
    2873 |                 new_order = min_order_for_split(folio);
         |                           ^


vim +/new_order +2517 mm/memory-failure.c

  2360	
  2361	/**
  2362	 * memory_failure - Handle memory failure of a page.
  2363	 * @pfn: Page Number of the corrupted page
  2364	 * @flags: fine tune action taken
  2365	 *
  2366	 * This function is called by the low level machine check code
  2367	 * of an architecture when it detects hardware memory corruption
  2368	 * of a page. It tries its best to recover, which includes
  2369	 * dropping pages, killing processes etc.
  2370	 *
  2371	 * The function is primarily of use for corruptions that
  2372	 * happen outside the current execution context (e.g. when
  2373	 * detected by a background scrubber)
  2374	 *
  2375	 * Must run in process context (e.g. a work queue) with interrupts
  2376	 * enabled and no spinlocks held.
  2377	 *
  2378	 * Return:
  2379	 *   0             - success,
  2380	 *   -ENXIO        - memory not managed by the kernel
  2381	 *   -EOPNOTSUPP   - hwpoison_filter() filtered the error event,
  2382	 *   -EHWPOISON    - the page was already poisoned, potentially
  2383	 *                   kill process,
  2384	 *   other negative values - failure.
  2385	 */
  2386	int memory_failure(unsigned long pfn, int flags)
  2387	{
  2388		struct page *p;
  2389		struct folio *folio;
  2390		struct dev_pagemap *pgmap;
  2391		int res = 0;
  2392		unsigned long page_flags;
  2393		bool retry = true;
  2394	
  2395		if (!sysctl_memory_failure_recovery)
  2396			panic("Memory failure on page %lx", pfn);
  2397	
  2398		mutex_lock(&mf_mutex);
  2399	
  2400		if (!(flags & MF_SW_SIMULATED))
  2401			hw_memory_failure = true;
  2402	
  2403		p = pfn_to_online_page(pfn);
  2404		if (!p) {
  2405			res = arch_memory_failure(pfn, flags);
  2406			if (res == 0)
  2407				goto unlock_mutex;
  2408	
  2409			if (!pfn_valid(pfn) && !arch_is_platform_page(PFN_PHYS(pfn))) {
  2410				/*
  2411				 * The PFN is not backed by struct page.
  2412				 */
  2413				res = memory_failure_pfn(pfn, flags);
  2414				goto unlock_mutex;
  2415			}
  2416	
  2417			if (pfn_valid(pfn)) {
  2418				pgmap = get_dev_pagemap(pfn);
  2419				put_ref_page(pfn, flags);
  2420				if (pgmap) {
  2421					res = memory_failure_dev_pagemap(pfn, flags,
  2422									 pgmap);
  2423					goto unlock_mutex;
  2424				}
  2425			}
  2426			pr_err("%#lx: memory outside kernel control\n", pfn);
  2427			res = -ENXIO;
  2428			goto unlock_mutex;
  2429		}
  2430	
  2431	try_again:
  2432		res = try_memory_failure_hugetlb(pfn, flags);
  2433		/*
  2434		 * -ENOENT means the page we found is not hugetlb, so proceed with normal page handling
  2435		 */
  2436		if (res != -ENOENT)
  2437			goto unlock_mutex;
  2438	
  2439		if (TestSetPageHWPoison(p)) {
  2440			res = -EHWPOISON;
  2441			if (flags & MF_ACTION_REQUIRED)
  2442				res = kill_accessing_process(current, pfn, flags);
  2443			if (flags & MF_COUNT_INCREASED)
  2444				put_page(p);
  2445			action_result(pfn, MF_MSG_ALREADY_POISONED, MF_FAILED);
  2446			goto unlock_mutex;
  2447		}
  2448	
  2449		/*
  2450		 * We need/can do nothing about count=0 pages.
  2451		 * 1) it's a free page, and therefore in safe hand:
  2452		 *    check_new_page() will be the gate keeper.
  2453		 * 2) it's part of a non-compound high order page.
  2454		 *    Implies some kernel user: cannot stop them from
  2455		 *    R/W the page; let's pray that the page has been
  2456		 *    used and will be freed some time later.
  2457		 * In fact it's dangerous to directly bump up page count from 0,
  2458		 * that may make page_ref_freeze()/page_ref_unfreeze() mismatch.
  2459		 */
  2460		res = get_hwpoison_page(p, flags);
  2461		switch (res) {
  2462		case 0:
  2463			if (is_free_buddy_page(p)) {
  2464				if (take_page_off_buddy(p)) {
  2465					page_ref_inc(p);
  2466					res = MF_RECOVERED;
  2467				} else {
  2468					/* We lost the race, try again */
  2469					if (retry) {
  2470						ClearPageHWPoison(p);
  2471						retry = false;
  2472						goto try_again;
  2473					}
  2474					res = MF_FAILED;
  2475				}
  2476				res = action_result(pfn, MF_MSG_BUDDY, res);
  2477			} else {
  2478				res = action_result(pfn, MF_MSG_KERNEL_HIGH_ORDER, MF_IGNORED);
  2479			}
  2480			goto unlock_mutex;
  2481		case 1:
  2482			/* Got a refcount on a handlable page. */
  2483			break;
  2484		case -ENOTRECOVERABLE:
  2485			/*
  2486			 * Stable unhandlable kernel-owned page (PG_reserved,
  2487			 * slab, page tables, large-kmalloc).
  2488			 * No recovery possible.
  2489			 */
  2490			res = action_result(pfn, MF_MSG_KERNEL, MF_IGNORED);
  2491			goto unlock_mutex;
  2492		default:
  2493			/* Transient lifecycle race with the page allocator. */
  2494			res = action_result(pfn, MF_MSG_GET_HWPOISON, MF_IGNORED);
  2495			goto unlock_mutex;
  2496		}
  2497	
  2498		folio = page_folio(p);
  2499	
  2500		/* filter pages that are protected from hwpoison test by users */
  2501		folio_lock(folio);
  2502		if (hwpoison_filter(p)) {
  2503			ClearPageHWPoison(p);
  2504			folio_unlock(folio);
  2505			folio_put(folio);
  2506			res = -EOPNOTSUPP;
  2507			goto unlock_mutex;
  2508		}
  2509	
  2510		folio_unlock(folio);
  2511	
  2512		if (folio_test_large(folio)) {
  2513			const int new_order;
  2514			int err;
  2515	
  2516			folio_lock(folio);
> 2517			new_order = min_order_for_split(folio);
  2518			folio_unlock(folio);
  2519	
  2520			/*
  2521			 * The flag must be set after the refcount is bumped
  2522			 * otherwise it may race with THP split.
  2523			 * And the flag can't be set in get_hwpoison_page() since
  2524			 * it is called by soft offline too and it is just called
  2525			 * for !MF_COUNT_INCREASED.  So here seems to be the best
  2526			 * place.
  2527			 *
  2528			 * Don't need care about the above error handling paths for
  2529			 * get_hwpoison_page() since they handle either free page
  2530			 * or unhandlable page.  The refcount is bumped iff the
  2531			 * page is a valid handlable page.
  2532			 */
  2533			folio_set_has_hwpoisoned(folio);
  2534			err = try_to_split_thp_page(p, new_order, /* release= */ false);
  2535			/*
  2536			 * If splitting a folio to order-0 fails, kill the process.
  2537			 * Split the folio regardless to minimize unusable pages.
  2538			 * Because the memory failure code cannot handle large
  2539			 * folios, this split is always treated as if it failed.
  2540			 */
  2541			if (err || new_order) {
  2542				/* get folio again in case the original one is split */
  2543				folio = page_folio(p);
  2544				res = -EHWPOISON;
  2545				kill_procs_now(p, pfn, flags, folio);
  2546				put_page(p);
  2547				action_result(pfn, MF_MSG_UNSPLIT_THP, MF_FAILED);
  2548				goto unlock_mutex;
  2549			}
  2550			VM_BUG_ON_PAGE(!page_count(p), p);
  2551			folio = page_folio(p);
  2552		}
  2553	
  2554		/*
  2555		 * We ignore non-LRU pages for good reasons.
  2556		 * - PG_locked is only well defined for LRU pages and a few others
  2557		 * - to avoid races with __SetPageLocked()
  2558		 * - to avoid races with __SetPageSlab*() (and more non-atomic ops)
  2559		 * The check (unnecessarily) ignores LRU pages being isolated and
  2560		 * walked by the page reclaim code, however that's not a big loss.
  2561		 */
  2562		shake_folio(folio);
  2563	
  2564		folio_lock(folio);
  2565	
  2566		/*
  2567		 * We're only intended to deal with the non-Compound page here.
  2568		 * The page cannot become compound pages again as folio has been
  2569		 * splited and extra refcnt is held.
  2570		 */
  2571		WARN_ON(folio_test_large(folio));
  2572	
  2573		/*
  2574		 * We use page flags to determine what action should be taken, but
  2575		 * the flags can be modified by the error containment action.  One
  2576		 * example is an mlocked page, where PG_mlocked is cleared by
  2577		 * folio_remove_rmap_*() in try_to_unmap_one(). So to determine page
  2578		 * status correctly, we save a copy of the page flags at this time.
  2579		 */
  2580		page_flags = folio->flags.f;
  2581	
  2582		/*
  2583		 * __munlock_folio() may clear a writeback folio's LRU flag without
  2584		 * the folio lock. We need to wait for writeback completion for this
  2585		 * folio or it may trigger a vfs BUG while evicting inode.
  2586		 */
  2587		if (!folio_test_lru(folio) && !folio_test_writeback(folio))
  2588			goto identify_page_state;
  2589	
  2590		/*
  2591		 * It's very difficult to mess with pages currently under IO
  2592		 * and in many cases impossible, so we just avoid it here.
  2593		 */
  2594		folio_wait_writeback(folio);
  2595	
  2596		/*
  2597		 * Now take care of user space mappings.
  2598		 * Abort on fail: __filemap_remove_folio() assumes unmapped page.
  2599		 */
  2600		if (!hwpoison_user_mappings(folio, p, pfn, flags)) {
  2601			res = action_result(pfn, MF_MSG_UNMAP_FAILED, MF_FAILED);
  2602			goto unlock_page;
  2603		}
  2604	
  2605		/*
  2606		 * Torn down by someone else?
  2607		 */
  2608		if (folio_test_lru(folio) && !folio_test_swapcache(folio) &&
  2609		    folio->mapping == NULL) {
  2610			res = action_result(pfn, MF_MSG_TRUNCATED_LRU, MF_IGNORED);
  2611			goto unlock_page;
  2612		}
  2613	
  2614	identify_page_state:
  2615		res = identify_page_state(pfn, p, page_flags);
  2616		mutex_unlock(&mf_mutex);
  2617		return res;
  2618	unlock_page:
  2619		folio_unlock(folio);
  2620	unlock_mutex:
  2621		mutex_unlock(&mf_mutex);
  2622		return res;
  2623	}
  2624	EXPORT_SYMBOL_GPL(memory_failure);
  2625	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-08-19 19:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  8:42 [PATCH v3] mm/memory-failure: fix concurrent access issue in min_order_for_split() dayou5941
2026-08-05  9:32 ` Lorenzo Stoakes (ARM)
2026-08-19 19:14 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox