Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756692AbdDRCrE (ORCPT ); Mon, 17 Apr 2017 22:47:04 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:36890 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756358AbdDRCrC (ORCPT ); Mon, 17 Apr 2017 22:47:02 -0400 Subject: Re: [PATCH V2] mm/madvise: Move up the behavior parameter validation To: Naoya Horiguchi , Anshuman Khandual References: <20170413092008.5437-1-khandual@linux.vnet.ibm.com> <20170414135141.15340-1-khandual@linux.vnet.ibm.com> <20170417052729.GA23423@hori1.linux.bs1.fc.nec.co.jp> Cc: "linux-kernel@vger.kernel.org" , "linux-mm@kvack.org" , "akpm@linux-foundation.org" From: Anshuman Khandual Date: Tue, 18 Apr 2017 08:16:53 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170417052729.GA23423@hori1.linux.bs1.fc.nec.co.jp> Content-Type: text/plain; charset=iso-2022-jp Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable x-cbid: 17041802-0012-0000-0000-000003E3FE17 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17041802-0013-0000-0000-00001B6AACDE Message-Id: X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-04-18_01:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1704180023 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1928 Lines: 55 On 04/17/2017 10:57 AM, Naoya Horiguchi wrote: > On Fri, Apr 14, 2017 at 07:21:41PM +0530, Anshuman Khandual wrote: >> The madvise_behavior_valid() function should be called before >> acting upon the behavior parameter. Hence move up the function. >> This also includes MADV_SOFT_OFFLINE and MADV_HWPOISON options >> as valid behavior parameter for the system call madvise(). >> >> Signed-off-by: Anshuman Khandual >> --- >> Changes in V2: >> >> Added CONFIG_MEMORY_FAILURE check before using MADV_SOFT_OFFLINE >> and MADV_HWPOISONE constants. >> >> mm/madvise.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/mm/madvise.c b/mm/madvise.c >> index efd4721..ccff186 100644 >> --- a/mm/madvise.c >> +++ b/mm/madvise.c >> @@ -694,6 +694,10 @@ static int madvise_inject_error(int behavior, >> #endif >> case MADV_DONTDUMP: >> case MADV_DODUMP: >> +#ifdef CONFIG_MEMORY_FAILURE >> + case MADV_SOFT_OFFLINE: >> + case MADV_HWPOISON: >> +#endif >> return true; >> >> default: >> @@ -767,12 +771,13 @@ static int madvise_inject_error(int behavior, >> size_t len; >> struct blk_plug plug; >> >> + if (!madvise_behavior_valid(behavior)) >> + return error; >> + >> #ifdef CONFIG_MEMORY_FAILURE >> if (behavior == MADV_HWPOISON || behavior == MADV_SOFT_OFFLINE) >> return madvise_inject_error(behavior, start, start + len_in); >> #endif >> - if (!madvise_behavior_valid(behavior)) >> - return error; > > Hi Anshuman, > > I'm wondering why current code calls madvise_inject_error() at the beginning > of SYSCALL_DEFINE3(madvise), without any boundary checks of address or length. > I agree to checking madvise_behavior_valid for MADV_{HWPOISON,SOFT_OFFLINE}, > but checking boundary of other arguments is also helpful, so how about moving > down the existing #ifdef block like below? Sure, will fold both the patches together and send it out.