All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <david@redhat.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: Miaohe Lin <linmiaohe@huawei.com>,
	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
Subject: Re: [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails
Date: Tue, 19 Apr 2022 13:14:29 +0200	[thread overview]
Message-ID: <21003e7a-01e4-c751-dd41-fce4149d424c@redhat.com> (raw)
In-Reply-To: <87pmldjxiq.fsf@nvdebian.thelocal>

On 19.04.22 10: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.

There is a difference.

What we want here is "there used to be something mapped but it's not
readable anymore. Please fail hard when userspace tries accessing
this.". Just like with hwpoison entries.

What a pte marker expresses is that "here is nothing mapped right now
but we have additional metadata available here. For file-backed memory,
it translates to: If we ever touch this page, lookup the pagecache what
to map here."

In the anonymous memory world, this would map to "populate the zeropage
or a fresh anonymous page on access." and keep the metadata around.

Yes, one might argue that for uffd-wp on anonymous memory it might make
sense to use pte marker as well, when no page has been populated yet.

IIRC, trying to arm uffd-wp when there is nothing populated yet will
simply get ignored.

> 
> 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.

We discussed using it for softdirty tracking as well.

-- 
Thanks,

David / dhildenb


  reply	other threads:[~2022-04-19 11:14 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 [this message]
2022-04-19 16:16           ` Peter Xu
2022-04-19 11:14         ` Miaohe Lin
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=21003e7a-01e4-c751-dd41-fce4149d424c@redhat.com \
    --to=david@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=dhowells@redhat.com \
    --cc=linmiaohe@huawei.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.