Re: [PATCH v2] mm/swapfile: unuse_pte can map random data if swap read fails

[David Hildenbrand <david@redhat.com>]

On 19.04.22 13:21, Miaohe Lin wrote:
> On 2022/4/19 15:37, David Hildenbrand wrote:
>> On 16.04.22 05:05, Miaohe Lin wrote:
>>> 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.
>>>
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>> ---
>>> v2:
>>>   use special swap entry to avoid permanently mounted swap
>>>   free the bad page in swapcache
>>> ---
>>>  include/linux/swap.h    |  7 ++++++-
>>>  include/linux/swapops.h | 10 ++++++++++
>>>  mm/memory.c             |  5 ++++-
>>>  mm/swapfile.c           | 11 +++++++++++
>>>  4 files changed, 31 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>>> index d112434f85df..03c576111737 100644
>>> --- a/include/linux/swap.h
>>> +++ b/include/linux/swap.h
>>> @@ -55,6 +55,10 @@ static inline int current_is_kswapd(void)
>>>   * actions on faults.
>>>   */
>>>  
>>> +#define SWAP_READ_ERROR_NUM 1
>>> +#define SWAP_READ_ERROR     (MAX_SWAPFILES + SWP_HWPOISON_NUM + \
>>> +			     SWP_MIGRATION_NUM + SWP_DEVICE_NUM + \
>>> +			     SWP_PTE_MARKER_NUM)
>>
>> Does anything speak against reusing the hwpoison marker? At least from a
>> program POV it's similar "the previously well defined content at this
>> user space address is no longer readable/writable".
> 
> Looks like a good idea. :)
> 
>>
>> I recall that we can just set the pfn to 0 for the hwpoison marker.
>>
>> There is e.g., check_hwpoisoned_entry() and it just stops if it finds
>> "pfn=0".
> 
> Do you mean that we should set the pfn to 0 for the hwpoison marker so that we can
> distinguish swapin error case from real hwpoison case?

I am not sure if we really have to distinguish. However, "0" seems to
make sense to indicate "this is not an actual problematic PFN, the
information is simply no longer around due to a hardware issue.

-- 
Thanks,

David / dhildenb