| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c61ca94b-5b19-4c69-b2a1-d11a5301c6bb@oracle.com>
Date: Mon, 29 Sep 2025 11:23:34 -0700
From: jane.chu@...cle.com
To: "Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>,
Zi Yan <ziy@...dia.com>
Cc: David Hildenbrand <david@...hat.com>,
Luis Chamberlain
<mcgrof@...nel.org>,
syzbot <syzbot+e6367ea2fdab6ed46056@...kaller.appspotmail.com>,
akpm@...ux-foundation.org, linmiaohe@...wei.com,
linux-kernel@...r.kernel.org, linux-mm@...ck.org,
nao.horiguchi@...il.com, syzkaller-bugs@...glegroups.com
Subject: Re: [syzbot] [mm?] WARNING in memory_failure
On 9/29/2025 10:49 AM, jane.chu@...cle.com wrote:
>
> On 9/29/2025 10:29 AM, jane.chu@...cle.com wrote:
>>
>> On 9/29/2025 4:08 AM, Pankaj Raghav (Samsung) wrote:
>>>>
>>>> I want to change all the split functions in huge_mm.h and provide
>>>> mapping_min_folio_order() to try_folio_split() in
>>>> truncate_inode_partial_folio().
>>>>
>>>> Something like below:
>>>>
>>>> 1. no split function will change the given order;
>>>> 2. __folio_split() will no longer give VM_WARN_ONCE when provided
>>>> new_order
>>>> is smaller than mapping_min_folio_order().
>>>>
>>>> In this way, for an LBS folio that cannot be split to order 0, split
>>>> functions will return -EINVAL to tell caller that the folio cannot
>>>> be split. The caller is supposed to handle the split failure.
>>>
>>> IIUC, we will remove warn on once but just return -EINVAL in
>>> __folio_split()
>>> function if new_order < min_order like this:
>>> ...
>>> min_order = mapping_min_folio_order(folio->mapping);
>>> if (new_order < min_order) {
>>> - VM_WARN_ONCE(1, "Cannot split mapped folio below min-
>>> order: %u",
>>> - min_order);
>>> ret = -EINVAL;
>>> goto out;
>>> }
>>> ...
>>
>> Then the user process will get a SIGBUS indicting the entire huge page
>> at higher order -
>> folio_set_has_hwpoisoned(folio);
>> if (try_to_split_thp_page(p, false) < 0) {
>> res = -EHWPOISON;
>> kill_procs_now(p, pfn, flags, folio);
>> put_page(p);
>> action_result(pfn, MF_MSG_UNSPLIT_THP,
>> MF_FAILED);
>> goto unlock_mutex;
>> }
>> VM_BUG_ON_PAGE(!page_count(p), p);
>> folio = page_folio(p);
>>
>> the huge page is not usable any way, kind of similar to the hugetlb
>> page situation: since the page cannot be splitted, the entire page is
>> marked unusable.
>>
>> How about keep the current huge page split code as is, but change the
>> M- F code to recognize that in a successful splitting case, the
>> poisoned page might just be in a lower folio order, and thus, deliver
>> the SIGBUS ?
>>
>> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
>> index a24806bb8e82..342c81edcdd9 100644
>> --- a/mm/memory-failure.c
>> +++ b/mm/memory-failure.c
>> @@ -2291,7 +2291,9 @@ int memory_failure(unsigned long pfn, int flags)
>> * page is a valid handlable page.
>> */
>> folio_set_has_hwpoisoned(folio);
>> - if (try_to_split_thp_page(p, false) < 0) {
>> + ret = try_to_split_thp_page(p, false);
>> + folio = page_folio(p);
>> + if (ret < 0 || folio_test_large(folio)) {
>> res = -EHWPOISON;
>> kill_procs_now(p, pfn, flags, folio);
>> put_page(p);
>> @@ -2299,7 +2301,6 @@ int memory_failure(unsigned long pfn, int flags)
>> goto unlock_mutex;
>> }
>> VM_BUG_ON_PAGE(!page_count(p), p);
>> - folio = page_folio(p);
>> }
>>
>> thanks,
>> -jane
>
> Maybe this is better, in case there are other reason for
> split_huge_page() to return -EINVAL.
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index a24806bb8e82..2bfa05acae65 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1659,9 +1659,10 @@ static int identify_page_state(unsigned long pfn,
> struct page *p,
> static int try_to_split_thp_page(struct page *page, bool release)
> {
> int ret;
> + int new_order = min_order_for_split(page_folio(page));
>
> lock_page(page);
> - ret = split_huge_page(page);
> + ret = split_huge_page_to_list_to_order(page, NULL, new_order);
> unlock_page(page);
>
> if (ret && release)
> @@ -2277,6 +2278,7 @@ int memory_failure(unsigned long pfn, int flags)
> folio_unlock(folio);
>
> if (folio_test_large(folio)) {
> + int ret;
> /*
> * The flag must be set after the refcount is bumped
> * otherwise it may race with THP split.
> @@ -2291,7 +2293,9 @@ int memory_failure(unsigned long pfn, int flags)
> * page is a valid handlable page.
> */
> folio_set_has_hwpoisoned(folio);
> - if (try_to_split_thp_page(p, false) < 0) {
> + ret = try_to_split_thp_page(p, false);
> + folio = page_folio(p);
> + if (ret < 0 || folio_test_large(folio)) {
> res = -EHWPOISON;
> kill_procs_now(p, pfn, flags, folio);
> put_page(p);
> @@ -2299,7 +2303,6 @@ int memory_failure(unsigned long pfn, int flags)
> goto unlock_mutex;
> }
> VM_BUG_ON_PAGE(!page_count(p), p);
> - folio = page_folio(p);
> }
>
> /*
> @@ -2618,7 +2621,8 @@ static int soft_offline_in_use_page(struct page
> *page)
> };
>
> if (!huge && folio_test_large(folio)) {
> - if (try_to_split_thp_page(page, true)) {
> + if ((try_to_split_thp_page(page, true)) ||
> + folio_test_large(page_folio(page))) {
> pr_info("%#lx: thp split failed\n", pfn);
> return -EBUSY;
> }
In soft offline, better to check if (min_order_for_split > 0), no need
to split, just return for now ...
thanks,
-jane
>
>
> thanks,
> -jane
>
>
Powered by blists - more mailing lists