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 <khand...@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.
