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 7558AC54EBE for ; Mon, 16 Jan 2023 20:51:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DA3C56B0071; Mon, 16 Jan 2023 15:51:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D533C6B0073; Mon, 16 Jan 2023 15:51:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C1B376B0074; Mon, 16 Jan 2023 15:51:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id AFA6A6B0071 for ; Mon, 16 Jan 2023 15:51:30 -0500 (EST) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 856981408D8 for ; Mon, 16 Jan 2023 20:51:30 +0000 (UTC) X-FDA: 80361857940.24.7718937 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf26.hostedemail.com (Postfix) with ESMTP id E51D8140010 for ; Mon, 16 Jan 2023 20:51:28 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=dKsb9xjJ; dmarc=none; spf=pass (imf26.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673902289; 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:dkim-signature; bh=tyoUzyH0Dk+6VAOclyyS2N51MWHiqnSzRtVoIS6GxAA=; b=yzY8GFvLl8ewyB4yej/nO4guwB40xfHZzl1ssRSqFkItV0tFYaK8hAtdORBD5CJ32MSlqs 8DAzuNKrzQH5Q/tIscbUlZaUjtMYxpt3uj2CwpdTm2Dp+t32MoxoywCTu+OntZhQ8aUuuP 82k7fl8QoaddNFdvL9aJ4pD8vIi4n1M= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=dKsb9xjJ; dmarc=none; spf=pass (imf26.hostedemail.com: domain of akpm@linux-foundation.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673902289; a=rsa-sha256; cv=none; b=kzCgGWnNoP6FoaalgXrn1mWas41eutuc8XkXfP3L2rTy/0ovuTFG3CzQUrRsQwX2VMjupP ebezIbrkNp6gFGIgdmA28jl+NWroe1udZGUhQvVxcYOVFcH6SPVEyMQUj2MHfQjVY1QdhL 0bmaFAfyh9yUkj13gS7I5d2i7LwPbVc= Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id AFB0D6110F; Mon, 16 Jan 2023 20:51:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id EA58AC433EF; Mon, 16 Jan 2023 20:51:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1673902287; bh=AJywCZGTrHNvlPQqOZFoEpIXxH9k1JRffYVtm1YaUzM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=dKsb9xjJll0S3VIrFztTMf6JzsJ7Ev+08a1vY64cR+/JbnoOuvgOBc33DkYHcifFv LQ4S5Kc8US8OCrabWe9MIeL/MzUBYx4lJTZI9AqUWDFScgAhyA1eLPnBqF+97pv/Kq OoM3gH2f3/xRGxjULH3Aoeae2toYKYH1FFNL6px8= Date: Mon, 16 Jan 2023 12:51:26 -0800 From: Andrew Morton To: Wupeng Ma Cc: , , , Subject: Re: [PATCH v2 1/4] mm/mlock: return EINVAL if len overflows for mlock/munlock Message-Id: <20230116125126.ed715ddf00ff4ffa2952ca29@linux-foundation.org> In-Reply-To: <20230116115813.2956935-2-mawupeng1@huawei.com> References: <20230116115813.2956935-1-mawupeng1@huawei.com> <20230116115813.2956935-2-mawupeng1@huawei.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: E51D8140010 X-Stat-Signature: k7z3rbkfu5ayikx6ersxe8nyhi4a4q67 X-HE-Tag: 1673902288-585592 X-HE-Meta: U2FsdGVkX1/00YUEbdNbn52I9SzWZoS7wA0eKemar+ToSw9MFNIZvc7vzt6qkWwt1SM1G7SA2NExK723LPvmxp6tFEIayrEFxlIj5zUkSnYXy4/7eD/L+WyXjOMYtWaE5GgowjG4xrKpMrgj38sk4YXe+/vuqtrsG1umrQn8vjB/O22MeSM0QMMNWwuKeu/HuY6NnluWLF5irXYZtkrPFYaZen4yQY9ivqYM/WkmrDi+K1Fwm7Q1HKkPmG2NT/pTBu1THFqUKD4zv/PjCQhLVsELfO7Dod93LpJMKsvvZJZcngBBI7mNwjChfIKgKsYFkXv4S3C61LXypF9hvPXK+Ydx+okCS7n7K0Le0WHRUUTMdW+/Y6SjWYRV9wtPSYB6MCVelIMCkufWAfrxaYd7ttSxPyoAblH4hiz14VlSMeGnMWCbQaJZ6DzIpeJiLFVIE2z4uNsuDcClVvXzXJQfnIAPfjeUcL3G8zwM7L07WpaKJmWA/SnopifcKNEGjiIv5xR0W6EWhC1Zbk2n2vYj4PhjWZ7DNhd2nV4bSL+QhOdBECDjVPMrdMgDvu9v30Xf6Sue0kHd+MgwxCzlyR28JYG5AgIc+zMcBLN8TCs+M8/H0hrMe7NPIh3Le+jhncM8SRWOevkcN+qsfa4oeJxCc8u4d/ykQyOmaupJt7Iey1ArwcPjmNQyvStPV5PtNH8QBQQaeWd1PFbHonuDqgQfHisxzcjJR1MVOJl79Cawt+GvanD0NHxdEQLZz1cixcpkyJvXkcW2QrApA2cHMTjPOomFohjy6hlbgSWsaB0ebL/yTg0RDEecjfTGMzJMSI9AvONdmr/bfRDBhSgqMMqIGIyup1mOUA7ubYB0U0dTyn0OUZMxagt4ctmpPMC5hq3wUnHw0YW/5Q1r0lNAHgc+6ucxRkAPqMzkv3Y/O+qxpcSAk3sTPdkgzEYppCjrOPQqMp0a9SqhzlFhRkDMKpv PN4jwL2z zPxJFcEFVQCSuVDXt81XF6Tj5ZqjaBNqDtjUK+hs/JPfckNmr36vtvz4uiCX1/A795T5c 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 Mon, 16 Jan 2023 19:58:10 +0800 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. > > ... > > --- a/mm/mlock.c > +++ b/mm/mlock.c > @@ -569,6 +569,7 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla > unsigned long locked; > unsigned long lock_limit; > int error = -ENOMEM; > + size_t old_len = len; I'm not sure that "old_len" is a good identifier. It reads to me like "the length of the old mlocked region" or something. I really don't like it when functions modify the values of the incoming argument like this. It would be better to leave `len' alone and create a new_len or something. > start = untagged_addr(start); > > @@ -578,6 +579,9 @@ static __must_check int do_mlock(unsigned long start, size_t len, vm_flags_t fla > len = PAGE_ALIGN(len + (offset_in_page(start))); > start &= PAGE_MASK; > > + if (old_len != 0 && len == 0) > + return -EINVAL; It would be clearer to do this immediately after calculating the new value of `len'. Before going on to play with `start'. Can we do something like this? --- a/mm/mlock.c~a +++ a/mm/mlock.c @@ -575,7 +575,12 @@ static __must_check int do_mlock(unsigne if (!can_do_mlock()) return -EPERM; - len = PAGE_ALIGN(len + (offset_in_page(start))); + if (len) { + len = PAGE_ALIGN(len + (offset_in_page(start))); + if (len == 0) /* overflow */ + return -EINVAL; + } + start &= PAGE_MASK; lock_limit = rlimit(RLIMIT_MEMLOCK); _ That depends on how we handle len==0. afaict, mlock(len==0) will presently burn a bunch of cpu cycles (not that we want to optimize this case), do nothing then return 0?