linux-kernel - Re: [PATCH 1/7] mm: shmem: correctly pass alloced parameter to shmem_recalc_inode() to avoid WARN

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: <cf70cde3-b4a4-4596-aefa-a510e082e129@linux.alibaba.com>
Date: Wed, 11 Jun 2025 15:29:40 +0800
From: Baolin Wang <baolin.wang@...ux.alibaba.com>
To: Kemeng Shi <shikemeng@...weicloud.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 1/7] mm: shmem: correctly pass alloced parameter to
 shmem_recalc_inode() to avoid WARN_ON()



On 2025/6/10 09:02, Kemeng Shi wrote:
> 
> 
> on 6/9/2025 8:46 AM, Kemeng Shi wrote:
>>
>>
>> on 6/7/2025 2:11 PM, Baolin Wang wrote:
>>>
>>>
>>> On 2025/6/6 06:10, Kemeng Shi wrote:
>>>> As noted in the comments, we need to release block usage for swap entry
>>>> which was replaced with poisoned swap entry. However, no block usage is
>>>> actually freed by calling shmem_recalc_inode(inode, -nr_pages, -nr_pages).
>>>> Instead, call shmem_recalc_inode(inode, 0, -nr_pages) can correctly release
>>>> the block usage.
>>>>
>>>> Fixes: 6cec2b95dadf7 ("mm/shmem: fix infinite loop when swap in shmem error at swapoff time")
>>>> Signed-off-by: Kemeng Shi <shikemeng@...weicloud.com>
>>>> ---
>>>>    mm/shmem.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/mm/shmem.c b/mm/shmem.c
>>>> index 4b42419ce6b2..e27d19867e03 100644
>>>> --- a/mm/shmem.c
>>>> +++ b/mm/shmem.c
>>>> @@ -2145,7 +2145,7 @@ static void shmem_set_folio_swapin_error(struct inode *inode, pgoff_t index,
>>>>         * won't be 0 when inode is released and thus trigger WARN_ON(i_blocks)
>>>>         * in shmem_evict_inode().
>>>>         */
>>>> -    shmem_recalc_inode(inode, -nr_pages, -nr_pages);
>>>> +    shmem_recalc_inode(inode, 0, -nr_pages);
>>>>        swap_free_nr(swap, nr_pages);
>>>>    }
>>>
>>> Have you tested your patch? When I inject an error to test your patch, the following issue will be triggered:As all issues are hard to trigger, I only run some simple test to ensure normal
>> process is fine. Could you share how to inject the error to trigger following
>> issue. I will have a deep look. Thanks
> Sorry that the message is truncated. I mean I only test normal process is fine.

Please also test the swapin error case you try to fix. Obviously your 
current patch is incorrect.

> Besides, I think there is another long-standing issue which could trigger the
> following issue. Here is the issue which is possible to blame:
> When swap entry is replaced with error entry in shmem_set_folio_swapin_error(),
> we will reduce info->swapped. Afterwards, error entry could be deleted in
> shmem_undo_range() and the info->swapped is reduced again. As a result, we
> reduce info->swapped twice for a single swap entry.

OK. So you should do something like in shmem_find_swap_entries() to 
avoid decreasing info->swapped again.

entry = radix_to_swp_entry(folio);
/*
* swapin error entries can be found in the mapping. But they're
* deliberately ignored here as we've done everything we can do.
*/
if (swp_type(entry) != type)
	continue;

> A simple way to confirm this is injecting error to original code. Could you
> share how to trigger the issue or could you do the same test to original code?

Yes, original code is good.

A simple test procedure is to allocate some shmem memory and swap them 
out, then swap in the shmem while injecting an error to trigger the 
swap-in error case, and finally unmap the program.