linux-kernel - Re: [PATCH 2/7] mm: shmem: avoid setting error on splited entries in shmem_set_folio_swapin

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]

Message-ID: <93336040-f457-d8a1-29df-f737efa8261c@huaweicloud.com>
Date: Wed, 11 Jun 2025 17:11:12 +0800
From: Kemeng Shi <shikemeng@...weicloud.com>
To: Baolin Wang <baolin.wang@...ux.alibaba.com>, hughd@...gle.com,
 willy@...radead.org, akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 2/7] mm: shmem: avoid setting error on splited entries in
 shmem_set_folio_swapin_error()



on 6/11/2025 3:41 PM, Baolin Wang wrote:
> 
> 
> On 2025/6/9 09:19, Kemeng Shi wrote:
>>
>>
>> on 6/7/2025 2:20 PM, Baolin Wang wrote:
>>>
>>>
>>> On 2025/6/6 06:10, Kemeng Shi wrote:
>>>> When large entry is splited, the first entry splited from large entry
>>>> retains the same entry value and index as original large entry but it's
>>>> order is reduced. In shmem_set_folio_swapin_error(), if large entry is
>>>> splited before xa_cmpxchg_irq(), we may replace the first splited entry
>>>> with error entry while using the size of original large entry for release
>>>> operations. This could lead to a WARN_ON(i_blocks) due to incorrect
>>>> nr_pages used by shmem_recalc_inode() and could lead to used after free
>>>> due to incorrect nr_pages used by swap_free_nr().
>>>
>>> I wonder if you have actually triggered this issue? When a large swap entry is split, it means the folio is already at order 0, so why would the size of the original large entry be used for release operations? Or is there another race condition?
>> All issues are found during review the code of shmem as I menthioned in
>> cover letter.
>> The folio could be allocated from shmem_swap_alloc_folio() and the folio
>> order will keep unchange when swap entry is split.
> 
> Sorry, I did not get your point. If a large swap entry is split, we must ensure that the corresponding folio is order 0.
> 
> However, I missed one potential case which was recently fixed by Kairui[1].
> 
> [1] https://lore.kernel.org/all/20250610181645.45922-1-ryncsn@gmail.com/
> 
Here is a possible code routine which I think could trigger the issue:
shmem_swapin_folio          shmem_swapin_folio
folio = swap_cache_get_folio()
order = xa_get_order(&mapping->i_pages, index);
if (!folio)
 ...
 /* suppose large folio allocation is failed, we will try to split large entry */
 folio = shmem_swap_alloc_folio(..., order, ...)

                            folio = swap_cache_get_folio()
                            order = xa_get_order(&mapping->i_pages, index);
                            if (!folio)
                             ...
                             /* suppose large folio allocation is successful this time */
                             folio = shmem_swap_alloc_folio(..., order, ...)
                            ...
                            /* suppose IO of large folio is failed, will set swapin error later */
                            if (!folio_test_uptodate(folio)) {
                             error = -EIO;
                             goto failed:
                            }

 ...
 shmem_split_large_entry()

                            ...
                            shmem_set_folio_swapin_error(..., folio, ...)