linux-kernel - Re: [PATCH -next resend v3] mm: hwposion: support recovery from ksm_might_need_to

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Feb 2023 09:23:27 +0800
From:   Kefeng Wang <wangkefeng.wang@...wei.com>
To:     Miaohe Lin <linmiaohe@...wei.com>,
        HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>
CC:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        David Hildenbrand <david@...hat.com>
Subject: Re: [PATCH -next resend v3] mm: hwposion: support recovery from
 ksm_might_need_to_copy()



On 2022/12/17 10:24, Miaohe Lin wrote:
> On 2022/12/16 9:47, HORIGUCHI NAOYA(堀口 直也) wrote:
>> On Tue, Dec 13, 2022 at 08:05:23PM +0800, Kefeng Wang wrote:
>>> When the kernel copy a page from ksm_might_need_to_copy(), but runs
>>> into an uncorrectable error, it will crash since poisoned page is
>>> consumed by kernel, this is similar to Copy-on-write poison recovery,
>>
>> Maybe you mean "this is similar to the issue recently fixed by
>> Copy-on-write poison recovery."?  And if this sentence ends here,
>> please put "." instead of ",".
>>
>>> When an error is detected during the page copy, return VM_FAULT_HWPOISON
>>> in do_swap_page(), and install a hwpoison entry in unuse_pte() when
>>> swapoff, which help us to avoid system crash.  Note, memory failure on
>>> a KSM page will be skipped, but still call memory_failure_queue() to
>>> be consistent with general memory failure process.
>>
>> Thank you for the work.  I have a few comment below ...
> 
> Thanks both.
> 
>>> -	if (unlikely(!PageUptodate(page))) {
>>> -		pte_t pteval;
>>> +	if (hwposioned || !PageUptodate(page)) {
>>> +		swp_entry_t swp_entry;
>>>   
>>>   		dec_mm_counter(vma->vm_mm, MM_SWAPENTS);
>>> -		pteval = swp_entry_to_pte(make_swapin_error_entry());
>>> -		set_pte_at(vma->vm_mm, addr, pte, pteval);
>>> -		swap_free(entry);
>>> +		if (hwposioned) {
>>> +			swp_entry = make_hwpoison_entry(swapcache);
>>> +			page = swapcache;
>>
>> This might work for the process accessing to the broken page, but ksm
>> pages are likely to be shared by multiple processes, so it would be
>> much nicer if you can convert all mapping entries for the error ksm page
>> into hwpoisoned ones.  Maybe in this thorough approach,
>> hwpoison_user_mappings() is updated to call try_to_unmap() for ksm pages.
>> But it's not necessary to do this together with applying mcsafe-memcpy,
>> because recovery action and mcsafe-memcpy can be done independently.
>>
> 
> I'm afraid leaving the ksm page in the cache will repeatedly trigger uncorrectable error for the
> same page if ksm pages are shared by multiple processes. This might reach the hardware threshold
> and result in fatal uncorrectable error (thus casuing system to panic). So IMHO it might be better
> to check if page is hwpoisoned before calling ksm_might_need_to_copy() if above thorough approach
> is not implemented. But I can easily be wrong.
> 
Oh，missing this one，there are only two caller, one in do_swap_page(),
it has already check whether the page is hwpoisoned or not, another is 
in unuse_pte() which only called from swapoff, it is not a hot patch, so
I don't think it is an urgent requirement. Thanks.