All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miaohe Lin <linmiaohe@huawei.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: <akpm@linux-foundation.org>, <willy@infradead.org>,
	<vbabka@suse.cz>, <dhowells@redhat.com>, <neilb@suse.de>,
	<surenb@google.com>, <minchan@kernel.org>, <peterx@redhat.com>,
	<sfr@canb.auug.org.au>, <rcampbell@nvidia.com>,
	<naoya.horiguchi@nec.com>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>,
	David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails
Date: Tue, 19 Apr 2022 19:14:40 +0800	[thread overview]
Message-ID: <535ee4f9-d8da-f8f6-2622-286f89622cb4@huawei.com> (raw)
In-Reply-To: <87pmldjxiq.fsf@nvdebian.thelocal>

On 2022/4/19 16:08, Alistair Popple wrote:
> David Hildenbrand <david@redhat.com> writes:
> 
>> On 19.04.22 09:29, Miaohe Lin wrote:
>>> On 2022/4/19 11:51, Alistair Popple wrote:
>>>> Miaohe Lin <linmiaohe@huawei.com> writes:
>>>>
>>>>> There is a bug in unuse_pte(): when swap page happens to be unreadable,
>>>>> page filled with random data is mapped into user address space. In case
>>>>> of error, a special swap entry indicating swap read fails is set to the
>>>>> page table. So the swapcache page can be freed and the user won't end up
>>>>> with a permanently mounted swap because a sector is bad. And if the page
>>>>> is accessed later, the user process will be killed so that corrupted data
>>>>> is never consumed. On the other hand, if the page is never accessed, the
>>>>> user won't even notice it.
>>>>
>>>> Hi Miaohe,
>>>>> It seems we're not actually using the pfn that gets stored in the special swap
>>>> entry here. Is my understanding correct? If so I think it would be better to use
>>>
>>> Yes, you're right. The pfn is not used now. What we need here is a special swap entry
>>> to do the right things. I think we can change to store some debugging information instead
>>> of pfn if needed in the future.
>>>
>>>> the new PTE markers Peter introduced[1] rather than adding another swap entry
>>>> type.
>>>
>>> IIUC, we should not reuse that swap entry here. From definition:
>>>
>>> PTE markers
>>> `========='
>>> ...
>>> PTE marker is a new type of swap entry that is ony applicable to file
>>> backed memories like shmem and hugetlbfs.  It's used to persist some
>>> pte-level information even if the original present ptes in pgtable are
>>> zapped.
>>>
>>> It's designed for file backed memories while swapin error entry is for anonymous
>>> memories. And there has some differences in processing. So it's not a good idea
>>> to reuse pte markers. Or am I miss something?
>>
>> I tend to agree. As raised in my other reply, maybe we can simply reuse
>> hwpoison entries and update the documentation of them accordingly.
> 
> Unless I've missed something I don't think PTE markers should be restricted
> solely to file backed memory. It's true that the only user of them at the moment
> is UFFD-WP for file backed memory, but PTE markers are just a special swap entry
> same as what is added here.
> 
> That said I don't think there has been any attempt to make PTE markers work for
> anything other than UFFD-WP because it was unclear if there ever would be
> another user.

If PTE markers can also handle the swapin error case, I will try to use it if
we can't reuse hwpoison entries.

> 
> But I agree re-using hwpoison entries is probably a better fit if possible.

Agree. As David said, "At least from a program POV it's similar "the previously well
defined content at this user space address is no longer readable/writable"."

> 
> - Alistair

Many thanks!

> 

  parent reply	other threads:[~2022-04-19 11:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-16  3:05 [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails Miaohe Lin
2022-04-19  3:51 ` Alistair Popple
2022-04-19  7:29   ` Miaohe Lin
2022-04-19  7:39     ` David Hildenbrand
2022-04-19  8:08       ` Alistair Popple
2022-04-19 11:14         ` David Hildenbrand
2022-04-19 16:16           ` Peter Xu
2022-04-19 11:14         ` Miaohe Lin [this message]
2022-04-19  7:53   ` Alistair Popple
2022-04-19 11:26     ` Miaohe Lin
2022-04-20  0:25       ` Alistair Popple
2022-04-20  6:15         ` Miaohe Lin
2022-04-20  7:07           ` David Hildenbrand
2022-04-20  8:37             ` Miaohe Lin
2022-04-19  7:37 ` David Hildenbrand
2022-04-19 11:21   ` Miaohe Lin
2022-04-19 11:46     ` David Hildenbrand
2022-04-19 12:00       ` Miaohe Lin
2022-04-19 12:12         ` David Hildenbrand
2022-04-19 12:45           ` Miaohe Lin
2022-04-19 21:36 ` Peter Xu
2022-04-20  5:56   ` [PATCH] mm/swap: Fix lost swap bits in unuse_pte() kernel test robot
2022-04-20  6:23     ` Miaohe Lin
2022-04-20  6:23       ` Miaohe Lin
2022-04-20  6:39       ` [kbuild-all] " Philip Li
2022-04-20  6:52         ` Miaohe Lin
2022-04-20  6:52           ` Miaohe Lin
2022-04-20  6:48       ` [kbuild-all] " Chen, Rong A
2022-04-20  6:56         ` Miaohe Lin
2022-04-20  6:56           ` Miaohe Lin
2022-04-20  6:21   ` [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails Miaohe Lin
2022-04-20 13:32     ` Peter Xu
2022-04-21  1:50       ` Miaohe Lin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=535ee4f9-d8da-f8f6-2622-286f89622cb4@huawei.com \
    --to=linmiaohe@huawei.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=david@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=naoya.horiguchi@nec.com \
    --cc=neilb@suse.de \
    --cc=peterx@redhat.com \
    --cc=rcampbell@nvidia.com \
    --cc=sfr@canb.auug.org.au \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.