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 kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E0569C05027 for ; Thu, 9 Feb 2023 11:35:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 56DC16B0071; Thu, 9 Feb 2023 06:35:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4F6EF6B0072; Thu, 9 Feb 2023 06:35:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 370F66B0074; Thu, 9 Feb 2023 06:35:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 212986B0071 for ; Thu, 9 Feb 2023 06:35:10 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id EBF75805DC for ; Thu, 9 Feb 2023 11:35:09 +0000 (UTC) X-FDA: 80447547138.18.10B865A Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf16.hostedemail.com (Postfix) with ESMTP id 359D9180009 for ; Thu, 9 Feb 2023 11:35:06 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=none; spf=pass (imf16.hostedemail.com: domain of mawupeng1@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=mawupeng1@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1675942508; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3wpmAIAjjijhpFnJa613mpx7I/oJfoMD9z+4+e2tMFg=; b=EsohVi5WZsJJqXJFIN7GsxyXciAGorSk5hBxcnM2jNPKjULb0eUgnEmGtv1Bm0tIKuBNq2 5CygRYANd5k3j9h7bsXZb4Jwv4EUZmC7800SEQw9Ksy88XO5RhxdaAB/KV+M59Vw7eVUXC vMGPGkyQlsBsRS5hS7qnYVG/RBCHNzk= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=none; spf=pass (imf16.hostedemail.com: domain of mawupeng1@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=mawupeng1@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1675942508; a=rsa-sha256; cv=none; b=7+ZV21B4QC/Zt9rrbABGipJCjSyXeTj6V8fbgY9iSlVxS5yu0BAlniNBuLHKeAzBSas/lm /FVEr7pGsyiaeBPb+pR6jNa1d2LXNdnCg0tZWEo3cWNnqqpW9wSkzZl8uluQ143cArZqIA L9haBKCVDpXrcR8eSTqqdNLw0f7XuIA= 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 X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: 5r56k7rwz17s9nrtcjntaoiq1pjesoim X-Rspamd-Queue-Id: 359D9180009 X-HE-Tag: 1675942506-379329 X-HE-Meta: U2FsdGVkX1+VRGdQQRiqTfzGBj+vNaQ7rU4oGPVK8Hk5H/MLRQ0oU2gLUcoiPXsxH/n1W59wVYaE1tWUZUL6sgoADlotZyRs824sVl2YKQUso9mBb1XxH+4vWKrikAvTriqah6tEyfesVTuqXRQ1ln9aUS/AiHLJO6EmpHBnyGEQgZU4J5oYZGBicbWa5aLiSj6okcxqRgXLH/nLFy9+FZ9LXLaHcKF1EWAee9BN3KztUtDuioObhBXk2KBubON6Q/BqSTq/hiAPbFihHbgnhngwOEnAnwzFFCc7i3p49+dTxWKU47gdl1KJ0Wnt4JmZOBfPwMZj+RNj6EpHfDznfGRf0TnV3QzSSdwJODgpHdPt3U00A7CYOCZwbR75nzbZH3/dJ82Jvb+5YiksNKHjicn0VuQ5N3fSYVfeSw8R3rGxPDQE97sUzlQp3AnrqPXti84JevKNwhX3+dHtFbdj6hwE6cSkPpK79THpmH5jU9n3fXDZZ1lgbKV/7zPiue/O5kECdUiLgkvN7pquU+2H+g2Hh+yrBTYCUSG45WK6+LrGIg4vJZBPpm8tsyFwEj7YGlPQlHUH9pWgAquR4OqHezcuhSbgQtXoFjdi7AJ4GnKQxX+NDCd2ynch/RKo6f0SE7XzAMADczunsWs26DXGsXuJ9dspJ51GTkwiSL4cSVYlPisQfxdnZiCvdDbiZl8OsSLJGVJ7aSMThsHNvPKaaDB7wrQDxhrSkDCS3AJNarzgnj2eRhoDwS6OK/PCfo7yfs0V7N/Y/mc3nT3IIbZNPF1wofUhuZph0MVokj0eOlXvz1C+ljJvFq3Ij8SKZ6HhwrLXrygih935U7QiWZDPbutBZ4QFzZBhAL+G1eZTuTuWVeEUSQCQH0qDfDW79aVRN3f799Fp5DYaAyy3YveE67FyCzl21S/KSGQjwfhbWzWnQQR6RRPFwrAyqONYxQBgepb8Upz5YxYAmArPqX9 1V4hBuq7 V2dUjfkJyKg7W76TjYdSiLNdMxSQQ6VeKEwsNqyq+T1FK4TkkbGNB8j5+RJoFsU4HmzhX9Qa5ubu5oTji58OinZaK51F8+R2KD/fGKQx/egVPyIsx4rPSaQb90HbiMDAveVwyKk2mM/Vg2LRugj2dDTy14QbAkJe2fhCZy3raoVjLxqs= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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. >