Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9583bc53-716d-f2ff-38e7-1dda7e57dd5d@redhat.com>
Date:   Wed, 22 Mar 2023 09:54:16 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.0
Subject: Re: [PATCH v4 1/4] mm/mlock: return EINVAL if len overflows for
 mlock/munlock
To:     mawupeng <mawupeng1@huawei.com>, akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        kuleshovmail@gmail.com, aneesh.kumar@linux.ibm.com
References: <20230320024739.224850-1-mawupeng1@huawei.com>
 <20230320024739.224850-2-mawupeng1@huawei.com>
 <27b9cb5b-0118-f989-80c2-6a143a4232af@redhat.com>
 <3ef9520c-6713-527a-0214-ac7a8bb2d49c@huawei.com>
 <6dd844f7-d43b-c744-f295-9f14c68d3928@redhat.com>
 <8be13253-b4ca-b134-3e85-b4097484bb29@huawei.com>
Content-Language: en-US
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
In-Reply-To: <8be13253-b4ca-b134-3e85-b4097484bb29@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 22.03.23 03:14, mawupeng wrote:
> 
> 
> On 2023/3/21 22:19, David Hildenbrand wrote:
>> On 21.03.23 08:44, mawupeng wrote:
>>>
>>>
>>> On 2023/3/20 18:54, David Hildenbrand wrote:
>>>> On 20.03.23 03:47, Wupeng Ma wrote:
>>>>> From: Ma Wupeng <mawupeng1@huawei.com>
>>>>>
>>>>> While testing mlock, we have a problem if the len of mlock is ULONG_MAX.
>>>>> The return value of mlock is zero. But nothing will be locked since the
>>>>> len in do_mlock overflows to zero due to the following code in mlock:
>>>>>
>>>>>      len = PAGE_ALIGN(len + (offset_in_page(start)));
>>>>>
>>>>> The same problem happens in munlock.
>>>>>
>>>>> Add new check and return -EINVAL to fix this overflowing scenarios since
>>>>> they are absolutely wrong.
>>>>
>>>> Thinking again, wouldn't we reject mlock(0, ULONG_MAX) now as well?
>>>
>>> mlock will return 0 if len is zero which is the same w/o this patchset.
>>> Here is the calltrace if len is zero.
>>>
>>> mlock(len == 0)
>>>      do_mlock(len == 0)
>>>          if (!len)
>>>              return 0
>>>
>>
>> I was asking about addr=0, len=ULONG_MAX.
>>
>> IIUC, that used to work but could now fail? I haven't played with it, though.
> 
> Thanks for reviewing.
> 
> Previous for add = 0 and len == ULONG_MAX, mlock will return ok(0) since len overflows to zero.
> IFAICT, this is not right since mlock do noting(lock nothing) and return ok(0).
> 
> With this patch, for the same situation, mlock can return EINVAL as expected.

Quoting the man page:

"EINVAL (mlock(),  mlock2(),  and  munlock()) The result of the addition 
addr+len was less than addr (e.g., the addition may have resulted in an 
overflow)."

ULONG_MAX+0 = ULONG_MAX

There is no overflow expected. The proper way to implement it would be 
to handle that case and not fail with EINVAL.

At least that would be expected when reading the man page.

-- 
Thanks,

David / dhildenb