From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 89B37C05027 for ; Thu, 9 Feb 2023 11:49:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229633AbjBILtJ (ORCPT ); Thu, 9 Feb 2023 06:49:09 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54906 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229596AbjBILs2 (ORCPT ); Thu, 9 Feb 2023 06:48:28 -0500 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7499357777 for ; Thu, 9 Feb 2023 03:35:26 -0800 (PST) Received: from dggpemm500014.china.huawei.com (unknown [172.30.72.57]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4PCF8v6qX3znVDt; Thu, 9 Feb 2023 19:32:47 +0800 (CST) Received: from [10.174.178.120] (10.174.178.120) by dggpemm500014.china.huawei.com (7.185.36.153) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Thu, 9 Feb 2023 19:35:01 +0800 Message-ID: <650460fa-7087-f7ab-6df9-3abc8b73c527@huawei.com> Date: Thu, 9 Feb 2023 19:35:00 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.0 From: mawupeng Subject: Re: [PATCH v3 1/4] mm/mlock: return EINVAL if len overflows for mlock/munlock To: , CC: , , , , References: <20230128063229.989058-1-mawupeng1@huawei.com> <20230128063229.989058-2-mawupeng1@huawei.com> <753c53d3-84a6-da73-4121-0db4a71e4fde@redhat.com> <10a3929a-7109-169f-6e42-e51c83305567@redhat.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.178.120] X-ClientProxiedBy: dggems703-chm.china.huawei.com (10.3.19.180) To dggpemm500014.china.huawei.com (7.185.36.153) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023/2/8 21:51, David Hildenbrand wrote: > On 07.02.23 02:24, mawupeng wrote: >> >> >> On 2023/2/7 1:05, David Hildenbrand wrote: >>> On 06.02.23 01:48, mawupeng wrote: >>>> >>>> >>>> On 2023/2/4 1:14, David Hildenbrand wrote: >>>>> On 28.01.23 07:32, Wupeng Ma wrote: >>>>>> From: Ma Wupeng >>>>>> >>>>>> 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. >>>>>> >>>>>> Return 0 early to avoid burn a bunch of cpu cycles if len == 0. >>>>>> >>>>>> Signed-off-by: Ma Wupeng >>>>>> --- >>>>>>     mm/mlock.c | 14 ++++++++++++-- >>>>>>     1 file changed, 12 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/mm/mlock.c b/mm/mlock.c >>>>>> index 7032f6dd0ce1..eb09968ba27f 100644 >>>>>> --- a/mm/mlock.c >>>>>> +++ b/mm/mlock.c >>>>>> @@ -478,8 +478,6 @@ static int apply_vma_lock_flags(unsigned long start, size_t len, >>>>>>         end = start + len; >>>>>>         if (end < start) >>>>>>             return -EINVAL; >>>>>> -    if (end == start) >>>>>> -        return 0; >>>>>>         vma = mas_walk(&mas); >>>>>>         if (!vma) >>>>>>             return -ENOMEM; >>>>>> @@ -575,7 +573,13 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla >>>>>>         if (!can_do_mlock()) >>>>>>             return -EPERM; >>>>>>     +    if (!len) >>>>>> +        return 0; >>>>>> + >>>>>>         len = PAGE_ALIGN(len + (offset_in_page(start))); >>>>>> +    if (!len) >>>>>> +        return -EINVAL; >>>>>> + >>>>>>         start &= PAGE_MASK; >>>>> >>>>> The "ordinary" overflows are detected in apply_vma_lock_flags(), correct? >>>> >>>> Overflow is not checked anywhere however the ordinary return early if len == 0 is detected in apply_vma_lock_flags(). >>>> >>> >>> I meant the >>> >>> end = start + len; >>> if (end < start) >>>      return -EINVAL; >>> >>> Essentially, what I wanted to double-check is that with your changes, we catch all kinds of overflows as documented in the man page, correct? >> >> Oh i see. You are right, The "ordinary" overflows are detected for mlock/munlock in apply_vma_lock_flags(). >> >> Yes, we may need to update the man page for all these four syscalls. > > E.g., mlock() already documents "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)." > > Just to rephrase my question what I wanted to double-check: are we now identifying all such overflows or are you aware of other corner cases? AFAICT. There is no cornel cases now. Pervious normal overflow can be detected via the following codes as you mentioned. end = start + len; if (end < start) return -EINVAL; this is fine for normal overflows. But the len may be zero in PAGE_ALIGN(len + (offset_in_page(start))). This leed to two scenarios for len == 0: a) user pass len as zero. this is fine, we don't need to do anything. b) overflow scenarios. we need to return err rather than response ok as above. >