linux-kernel - Re: [PATCH] mm/mremap: avoid unneeded do

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] [day] [month] [year] [list]

Message-ID: <18cb65e5-e124-57be-31ed-9ea227c869df@huawei.com>
Date:   Sat, 2 Apr 2022 09:52:27 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     David Hildenbrand <david@...hat.com>, <akpm@...ux-foundation.org>
CC:     <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/mremap: avoid unneeded do_munmap call

On 2022/4/1 18:38, David Hildenbrand wrote:
> On 01.04.22 10:10, Miaohe Lin wrote:
>> When old_len == new_len, do_munmap will return -EINVAL due to len == 0.
>> This errno will be simply ignored because of old_len != new_len check.
>> So it is unnecessary to call do_munmap when old_len == new_len because
>> nothing is actually done.
>>
>> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
>> ---
>>  mm/mremap.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/mremap.c b/mm/mremap.c
>> index e776d4c2345c..dd966621a056 100644
>> --- a/mm/mremap.c
>> +++ b/mm/mremap.c
>> @@ -817,9 +817,9 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>>  			goto out;
>>  	}
>>  
>> -	if (old_len >= new_len) {
>> +	if (old_len > new_len) {
>>  		ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
>> -		if (ret && old_len != new_len)
>> +		if (ret)
>>  			goto out;
>>  		old_len = new_len;
>>  	}
> 
> I remember stumbling over that myself a year ago or so but dig not
> deeper. But indeed, both variants (mmu, nommu) return -EINVAL in case
> len (old_len - new_len) == 0.
> 
> Maybe that used to be different before ecc1a8993751 ("do_mremap()
> untangling, part 2"), but it doesn't look like it.

It could be really hard to trace back to the reason. But it's clear that
this change does the same thing now and simplifies the code a bit.

> 
> Acked-by: David Hildenbrand <david@...hat.com>

Many thanks for your comment and Acked-by tag! Have a nice weekend! :)

>