* [PATCH] mm/madvise: Move up the behavior parameter validation @ 2017-04-13 9:20 Anshuman Khandual 2017-04-13 13:59 ` kbuild test robot ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Anshuman Khandual @ 2017-04-13 9:20 UTC (permalink / raw) To: linux-kernel, linux-mm; +Cc: n-horiguchi, akpm 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 <khandual@linux.vnet.ibm.com> --- This applies on top of the other madvise clean up patch I sent earlier this week https://patchwork.kernel.org/patch/9672095/ mm/madvise.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/madvise.c b/mm/madvise.c index efd4721..3cb427a 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -694,6 +694,8 @@ static int madvise_inject_error(int behavior, #endif case MADV_DONTDUMP: case MADV_DODUMP: + case MADV_SOFT_OFFLINE: + case MADV_HWPOISON: return true; default: @@ -767,12 +769,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; if (start & ~PAGE_MASK) return error; -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] mm/madvise: Move up the behavior parameter validation 2017-04-13 9:20 [PATCH] mm/madvise: Move up the behavior parameter validation Anshuman Khandual @ 2017-04-13 13:59 ` kbuild test robot 2017-04-14 13:51 ` [PATCH V2] " Anshuman Khandual 2017-04-18 5:28 ` [PATCH V3] " Anshuman Khandual 2 siblings, 0 replies; 8+ messages in thread From: kbuild test robot @ 2017-04-13 13:59 UTC (permalink / raw) To: Anshuman Khandual; +Cc: kbuild-all, linux-kernel, linux-mm, n-horiguchi, akpm [-- Attachment #1: Type: text/plain, Size: 1706 bytes --] Hi Anshuman, [auto build test ERROR on next-20170412] [also build test ERROR on v4.11-rc6] [cannot apply to mmotm/master v4.9-rc8 v4.9-rc7 v4.9-rc6] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Anshuman-Khandual/mm-madvise-Move-up-the-behavior-parameter-validation/20170413-203541 config: parisc-c3000_defconfig (attached as .config) compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=parisc All errors (new ones prefixed by >>): mm/madvise.c: In function 'madvise_behavior_valid': >> mm/madvise.c:690:7: error: 'MADV_SOFT_OFFLINE' undeclared (first use in this function) case MADV_SOFT_OFFLINE: ^~~~~~~~~~~~~~~~~ mm/madvise.c:690:7: note: each undeclared identifier is reported only once for each function it appears in >> mm/madvise.c:691:7: error: 'MADV_HWPOISON' undeclared (first use in this function) case MADV_HWPOISON: ^~~~~~~~~~~~~ vim +/MADV_SOFT_OFFLINE +690 mm/madvise.c 684 #ifdef CONFIG_TRANSPARENT_HUGEPAGE 685 case MADV_HUGEPAGE: 686 case MADV_NOHUGEPAGE: 687 #endif 688 case MADV_DONTDUMP: 689 case MADV_DODUMP: > 690 case MADV_SOFT_OFFLINE: > 691 case MADV_HWPOISON: 692 return true; 693 694 default: --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 14651 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V2] mm/madvise: Move up the behavior parameter validation 2017-04-13 9:20 [PATCH] mm/madvise: Move up the behavior parameter validation Anshuman Khandual 2017-04-13 13:59 ` kbuild test robot @ 2017-04-14 13:51 ` Anshuman Khandual 2017-04-17 5:27 ` Naoya Horiguchi 2017-04-18 5:28 ` [PATCH V3] " Anshuman Khandual 2 siblings, 1 reply; 8+ messages in thread From: Anshuman Khandual @ 2017-04-14 13:51 UTC (permalink / raw) To: linux-kernel, linux-mm; +Cc: n-horiguchi, akpm 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 <khandual@linux.vnet.ibm.com> --- 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; if (start & ~PAGE_MASK) return error; -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] mm/madvise: Move up the behavior parameter validation 2017-04-14 13:51 ` [PATCH V2] " Anshuman Khandual @ 2017-04-17 5:27 ` Naoya Horiguchi 2017-04-18 2:46 ` Anshuman Khandual 0 siblings, 1 reply; 8+ messages in thread From: Naoya Horiguchi @ 2017-04-17 5:27 UTC (permalink / raw) To: Anshuman Khandual; +Cc: linux-kernel, linux-mm, akpm 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 <khandual@linux.vnet.ibm.com> > --- > 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? diff --git a/mm/madvise.c b/mm/madvise.c index a09d2d3dfae9..7b36912e1f4a 100644 --- a/mm/madvise.c +++ b/mm/madvise.c @@ -754,10 +754,6 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior) size_t len; struct blk_plug plug; -#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; @@ -777,6 +773,11 @@ SYSCALL_DEFINE3(madvise, unsigned long, start, size_t, len_in, int, behavior) if (end == start) return error; +#ifdef CONFIG_MEMORY_FAILURE + if (behavior == MADV_HWPOISON || behavior == MADV_SOFT_OFFLINE) + return madvise_inject_error(behavior, start, start+len_in); +#endif + write = madvise_need_mmap_write(behavior); if (write) { if (down_write_killable(¤t->mm->mmap_sem)) Thanks, Naoya Horiguchi ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] mm/madvise: Move up the behavior parameter validation 2017-04-17 5:27 ` Naoya Horiguchi @ 2017-04-18 2:46 ` Anshuman Khandual 0 siblings, 0 replies; 8+ messages in thread From: Anshuman Khandual @ 2017-04-18 2:46 UTC (permalink / raw) To: Naoya Horiguchi, Anshuman Khandual; +Cc: linux-kernel, linux-mm, akpm 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 <khandual@linux.vnet.ibm.com> >> --- >> 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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH V3] mm/madvise: Move up the behavior parameter validation 2017-04-13 9:20 [PATCH] mm/madvise: Move up the behavior parameter validation Anshuman Khandual 2017-04-13 13:59 ` kbuild test robot 2017-04-14 13:51 ` [PATCH V2] " Anshuman Khandual @ 2017-04-18 5:28 ` Anshuman Khandual 2017-04-18 21:01 ` David Rientjes 2017-04-19 0:46 ` Naoya Horiguchi 2 siblings, 2 replies; 8+ messages in thread From: Anshuman Khandual @ 2017-04-18 5:28 UTC (permalink / raw) To: linux-kernel, linux-mm; +Cc: n-horiguchi, akpm 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 <khandual@linux.vnet.ibm.com> --- Changes in V3: Moved the madvise_inject_error() function down which will make sure that the boundary conditions are checked for address and length arguments as per Naoya. Changes in V2: Added CONFIG_MEMORY_FAILURE check before using MADV_SOFT_OFFLINE and MADV_HWPOISONE constants. mm/madvise.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/mm/madvise.c b/mm/madvise.c index efd4721..721dd6f 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,10 +771,6 @@ static int madvise_inject_error(int behavior, size_t len; struct blk_plug plug; -#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; @@ -790,6 +790,11 @@ static int madvise_inject_error(int behavior, if (end == start) return error; +#ifdef CONFIG_MEMORY_FAILURE + if (behavior == MADV_HWPOISON || behavior == MADV_SOFT_OFFLINE) + return madvise_inject_error(behavior, start, start + len_in); +#endif + write = madvise_need_mmap_write(behavior); if (write) { if (down_write_killable(¤t->mm->mmap_sem)) -- 1.8.5.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V3] mm/madvise: Move up the behavior parameter validation 2017-04-18 5:28 ` [PATCH V3] " Anshuman Khandual @ 2017-04-18 21:01 ` David Rientjes 2017-04-19 0:46 ` Naoya Horiguchi 1 sibling, 0 replies; 8+ messages in thread From: David Rientjes @ 2017-04-18 21:01 UTC (permalink / raw) To: Anshuman Khandual; +Cc: linux-kernel, linux-mm, n-horiguchi, akpm On Tue, 18 Apr 2017, 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 <khandual@linux.vnet.ibm.com> Acked-by: David Rientjes <rientjes@google.com> Looks like this depends on existing patches in -mm. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V3] mm/madvise: Move up the behavior parameter validation 2017-04-18 5:28 ` [PATCH V3] " Anshuman Khandual 2017-04-18 21:01 ` David Rientjes @ 2017-04-19 0:46 ` Naoya Horiguchi 1 sibling, 0 replies; 8+ messages in thread From: Naoya Horiguchi @ 2017-04-19 0:46 UTC (permalink / raw) To: Anshuman Khandual; +Cc: linux-kernel, linux-mm, akpm On Tue, Apr 18, 2017 at 10:58:44AM +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 <khandual@linux.vnet.ibm.com> Acked-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-19 0:47 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-13 9:20 [PATCH] mm/madvise: Move up the behavior parameter validation Anshuman Khandual 2017-04-13 13:59 ` kbuild test robot 2017-04-14 13:51 ` [PATCH V2] " Anshuman Khandual 2017-04-17 5:27 ` Naoya Horiguchi 2017-04-18 2:46 ` Anshuman Khandual 2017-04-18 5:28 ` [PATCH V3] " Anshuman Khandual 2017-04-18 21:01 ` David Rientjes 2017-04-19 0:46 ` Naoya Horiguchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).