linux-kernel - Re: [PATCH v4 2/2] mm, thp: bail out early in collapse

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:   Mon, 11 Oct 2021 11:22:14 +0800
From:   Rongwei Wang <rongwei.wang@...ux.alibaba.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        akpm@...ux-foundation.org, song@...nel.org,
        william.kucharski@...cle.com, hughd@...gle.com, shy828301@...il.com
Subject: Re: [PATCH v4 2/2] mm, thp: bail out early in collapse_file for
 writeback page



On 10/11/21 11:08 AM, Matthew Wilcox wrote:
> On Mon, Oct 11, 2021 at 10:22:41AM +0800, Rongwei Wang wrote:
>> Currently collapse_file does not explicitly check PG_writeback, instead,
>> page_has_private and try_to_release_page are used to filter writeback
>> pages. This does not work for xfs with blocksize equal to or larger
>> than pagesize, because in such case xfs has no page->private.
>>
>> This makes collapse_file bail out early for writeback page. Otherwise,
>> xfs end_page_writeback will panic as follows.
>>
>> Fixes: eb6ecbed0aa2 ("mm, thp: relax the VM_DENYWRITE constraint on file-backed THPs")
> 
> This is the wrong Fixes line.  This same bug exists earlier than this.
> Your testing may not show it before then, but if you mmap something
> that isn't an executable, you can provoke it.  It should be:
> 
> Fixes: 99cb0dbd47a1 ("mm,thp: add read-only THP support for (non-shmem) FS")
> 
> (unless there's something I'm missing?)
Hi, Matthew
I forget the Patch #2 fix the bug that is different with Patch #1. I 
will update this.

Thanks for your remind!
> 
> Also, this should surely have a Cc: stable@...r.kernel.org in the
> tags section?  It's a user-visible bug, we want it backported.
OK, Thanks!
> 
>> Suggested-by: Yang Shi <shy828301@...il.com>
>> Signed-off-by: Xu Yu <xuyu@...ux.alibaba.com>
>> Signed-off-by: Rongwei Wang <rongwei.wang@...ux.alibaba.com>
>> Reviewed-by: Matthew Wilcox (Oracle) <willy@...radead.org>
>> Reviewed-by: Yang Shi <shy828301@...il.com>
>> ---
>>   mm/khugepaged.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index 045cc579f724..48de4e1b0783 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -1763,6 +1763,10 @@ static void collapse_file(struct mm_struct *mm,
>>   				filemap_flush(mapping);
>>   				result = SCAN_FAIL;
>>   				goto xa_unlocked;
>> +			} else if (PageWriteback(page)) {
>> +				xas_unlock_irq(&xas);
>> +				result = SCAN_FAIL;
>> +				goto xa_unlocked;
>>   			} else if (trylock_page(page)) {
>>   				get_page(page);
>>   				xas_unlock_irq(&xas);
>> @@ -1798,7 +1802,8 @@ static void collapse_file(struct mm_struct *mm,
>>   			goto out_unlock;
>>   		}
>>   
>> -		if (!is_shmem && PageDirty(page)) {
>> +		if (!is_shmem && (PageDirty(page) ||
>> +				  PageWriteback(page))) {
>>   			/*
>>   			 * khugepaged only works on read-only fd, so this
>>   			 * page is dirty because it hasn't been flushed
>> -- 
>> 2.27.0
>>