| 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: <b451c3b8-98d8-4df7-b076-cf9e6ed7a087@oracle.com>
Date: Wed, 1 Oct 2025 11:15:52 -0700
From: jane.chu@...cle.com
To: Miaohe Lin <linmiaohe@...wei.com>
Cc: David Hildenbrand <david@...hat.com>,
Luis Chamberlain
<mcgrof@...nel.org>,
syzbot <syzbot+e6367ea2fdab6ed46056@...kaller.appspotmail.com>,
akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, nao.horiguchi@...il.com,
syzkaller-bugs@...glegroups.com,
"Pankaj Raghav (Samsung)" <kernel@...kajraghav.com>,
Zi Yan <ziy@...dia.com>
Subject: Re: [syzbot] [mm?] WARNING in memory_failure
On 9/29/2025 11:31 PM, Miaohe Lin wrote:
> On 2025/9/30 12:35, jane.chu@...cle.com wrote:
>>
>>
>> On 9/29/2025 7:51 PM, Miaohe Lin wrote:
>>> On 2025/9/30 2:23, jane.chu@...cle.com wrote:
>>>>
>>>>
>>>> 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 ...
>>>
>>> I might be miss something but why we have to split it? Could we migrate the whole thp or folio with min_order instead?
>>
>> The soft offline code was originally written with the assumption that only 1 base page will be offlined.
>
> Yes, only page corresponding to parameter @pfn of soft_offline_page() will be offlined.
>
>>
>> With the recent introduction of min_order, it might quietly offline multiple pages, is that a desirable thing?
>
> I don't think so. Even if try_to_split_thp_page splits folio into smaller one with min_order, page_handle_poison()
> will put back the folio into buddy after migrate_pages, set the hwpoisoned flag to raw error page and hold the extra
> refcnt. So only raw error page will be offlined while other sub-pages will be put back into buddy.
> Or am I miss something?
The thing is that the non-poisoned subpages are also migrated away,
they're no longer available for the user process.
thanks,
-jane
>
> Thanks.
> .
Powered by blists - more mailing lists