linux-kernel - Re: [PATCH v2] mm: zswap: handle incorrect attempts to load of large folios

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: Tue, 11 Jun 2024 07:14:19 +0300
From: Mika Penttilä <mpenttil@...hat.com>
To: Yosry Ahmed <yosryahmed@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
 Johannes Weiner <hannes@...xchg.org>, Nhat Pham <nphamcs@...il.com>,
 Chengming Zhou <chengming.zhou@...ux.dev>,
 Baolin Wang <baolin.wang@...ux.alibaba.com>, Barry Song <21cnbao@...il.com>,
 Chris Li <chrisl@...nel.org>, Ryan Roberts <ryan.roberts@....com>,
 David Hildenbrand <david@...hat.com>, Matthew Wilcox <willy@...radead.org>,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mm: zswap: handle incorrect attempts to load of large
 folios


On 6/10/24 20:35, Yosry Ahmed wrote:
> On Fri, Jun 7, 2024 at 9:08 PM Mika Penttilä <mpenttil@...hat.com> wrote:
>> On 6/8/24 05:36, Yosry Ahmed wrote:
>>> diff --git a/mm/zswap.c b/mm/zswap.c
>>> index b9b35ef86d9be..ebb878d3e7865 100644
>>> --- a/mm/zswap.c
>>> +++ b/mm/zswap.c
>>> @@ -1557,6 +1557,26 @@ bool zswap_load(struct folio *folio)
>>>
>>>       VM_WARN_ON_ONCE(!folio_test_locked(folio));
>>>
>>> +     /*
>>> +      * Large folios should not be swapped in while zswap is being used, as
>>> +      * they are not properly handled. Zswap does not properly load large
>>> +      * folios, and a large folio may only be partially in zswap.
>>> +      *
>>> +      * If any of the subpages are in zswap, reading from disk would result
>>> +      * in data corruption, so return true without marking the folio uptodate
>>> +      * so that an IO error is emitted (e.g. do_swap_page() will sigfault).
>>> +      *
>>> +      * Otherwise, return false and read the folio from disk.
>>> +      */
>>> +     if (folio_test_large(folio)) {
>>> +             if (xa_find(tree, &offset,
>>> +                         offset + folio_nr_pages(folio) - 1, XA_PRESENT)) {
>>> +                     WARN_ON_ONCE(1);
>>> +                     return true;
>>> +             }
>> How does that work? Should it be xa_find_after() to not always find
>> current entry?
> By "current entry" I believe you mean the entry corresponding to
> "offset" (i.e. the first subpage of the folio). At this point, we
> haven't checked if that offset has a corresponding entry in zswap or
> not. It may be on disk, or zwap may be disabled.

Okay you test if there's any matching offset in zswap for the folio.


>> And does it still mean those subsequent entries map to same folio?
> If I understand correctly, a folio in the swapcache has contiguous
> swap offsets for its subpages. So I am assuming that the large folio
> swapin case will adhere to that (i.e. we only swapin a large folio if
> the swap offsets are contiguous). Did I misunderstand something here?

Yes I think that is fair assumption for now. But also saw your v3 which
doesn't depend on this.


>
>>
>> --Mika
>>
>>