* [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
@ 2019-10-30 16:58 Yang Shi
2019-10-30 18:14 ` Yang Shi
0 siblings, 1 reply; 6+ messages in thread
From: Yang Shi @ 2019-10-30 16:58 UTC (permalink / raw)
To: lixinhai.lxh, vbabka, mhocko, mgorman, akpm
Cc: yang.shi, stable, linux-mm, linux-kernel
The commit d883544515aa ("mm: mempolicy: make the behavior consistent
when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
value of mbind() for a couple of corner cases. But, it altered the
errno for some other cases, for example, mbind() should return -EFAULT
when part or all of the memory range specified by nodemask and maxnode
points outside your accessible address space, or there was an unmapped
hole in the specified memory range specified by addr and len.
Fixed this by preserving the errno returned by queue_pages_range().
And, the pagelist may be not empty even though queue_pages_range()
returns error, put the pages back to LRU since mbind_range() is not called
to really apply the policy so those pages should not be migrated, this
is also the old behavior before the problematic commit.
Reported-by: Li Xinhai <lixinhai.lxh@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: <stable@vger.kernel.org> v4.19 and v5.2+
Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
---
mm/mempolicy.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 4ae967b..e08c941 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -672,7 +672,9 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
* 1 - there is unmovable page, but MPOL_MF_MOVE* & MPOL_MF_STRICT were
* specified.
* 0 - queue pages successfully or no misplaced page.
- * -EIO - there is misplaced page and only MPOL_MF_STRICT was specified.
+ * errno - i.e. misplaced pages with MPOL_MF_STRICT specified (-EIO) or
+ * memory range specified by nodemask and maxnode points outside
+ * your accessible address space (-EFAULT)
*/
static int
queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
@@ -1286,7 +1288,7 @@ static long do_mbind(unsigned long start, unsigned long len,
flags | MPOL_MF_INVERT, &pagelist);
if (ret < 0) {
- err = -EIO;
+ err = ret;
goto up_out;
}
@@ -1305,10 +1307,12 @@ static long do_mbind(unsigned long start, unsigned long len,
if ((ret > 0) || (nr_failed && (flags & MPOL_MF_STRICT)))
err = -EIO;
- } else
- putback_movable_pages(&pagelist);
-
+ } else {
up_out:
+ if (!list_empty(&pagelist))
+ putback_movable_pages(&pagelist);
+ }
+
up_write(&mm->mmap_sem);
mpol_out:
mpol_put(new);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
2019-10-30 16:58 [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind Yang Shi
@ 2019-10-30 18:14 ` Yang Shi
2019-10-31 1:53 ` Li Xinhai
2019-10-31 4:31 ` Andrew Morton
0 siblings, 2 replies; 6+ messages in thread
From: Yang Shi @ 2019-10-30 18:14 UTC (permalink / raw)
To: lixinhai.lxh, vbabka, mhocko, mgorman, akpm
Cc: stable, linux-mm, linux-kernel
On 10/30/19 9:58 AM, Yang Shi wrote:
> The commit d883544515aa ("mm: mempolicy: make the behavior consistent
> when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
> value of mbind() for a couple of corner cases. But, it altered the
> errno for some other cases, for example, mbind() should return -EFAULT
> when part or all of the memory range specified by nodemask and maxnode
> points outside your accessible address space, or there was an unmapped
> hole in the specified memory range specified by addr and len.
>
> Fixed this by preserving the errno returned by queue_pages_range().
> And, the pagelist may be not empty even though queue_pages_range()
> returns error, put the pages back to LRU since mbind_range() is not called
> to really apply the policy so those pages should not be migrated, this
> is also the old behavior before the problematic commit.
Forgot fixes tag.
Fixes: d883544515aa ("mm: mempolicy: make the behavior consistent when
MPOL_MF_MOVE* and MPOL_MF_STRICT were specified")
> Reported-by: Li Xinhai <lixinhai.lxh@gmail.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: <stable@vger.kernel.org> v4.19 and v5.2+
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> ---
> mm/mempolicy.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4ae967b..e08c941 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -672,7 +672,9 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
> * 1 - there is unmovable page, but MPOL_MF_MOVE* & MPOL_MF_STRICT were
> * specified.
> * 0 - queue pages successfully or no misplaced page.
> - * -EIO - there is misplaced page and only MPOL_MF_STRICT was specified.
> + * errno - i.e. misplaced pages with MPOL_MF_STRICT specified (-EIO) or
> + * memory range specified by nodemask and maxnode points outside
> + * your accessible address space (-EFAULT)
> */
> static int
> queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
> @@ -1286,7 +1288,7 @@ static long do_mbind(unsigned long start, unsigned long len,
> flags | MPOL_MF_INVERT, &pagelist);
>
> if (ret < 0) {
> - err = -EIO;
> + err = ret;
> goto up_out;
> }
>
> @@ -1305,10 +1307,12 @@ static long do_mbind(unsigned long start, unsigned long len,
>
> if ((ret > 0) || (nr_failed && (flags & MPOL_MF_STRICT)))
> err = -EIO;
> - } else
> - putback_movable_pages(&pagelist);
> -
> + } else {
> up_out:
> + if (!list_empty(&pagelist))
> + putback_movable_pages(&pagelist);
> + }
> +
> up_write(&mm->mmap_sem);
> mpol_out:
> mpol_put(new);
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Re: [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
2019-10-30 18:14 ` Yang Shi
@ 2019-10-31 1:53 ` Li Xinhai
2019-10-31 4:31 ` Andrew Morton
1 sibling, 0 replies; 6+ messages in thread
From: Li Xinhai @ 2019-10-31 1:53 UTC (permalink / raw)
To: yang.shi, Vlastimil Babka, mhocko, mgorman, akpm
Cc: stable, linux-mm, linux-kernel
On 2019-10-31 at 02:14 Yang Shi wrote:
>
>
>On 10/30/19 9:58 AM, Yang Shi wrote:
>> The commit d883544515aa ("mm: mempolicy: make the behavior consistent
>> when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
>> value of mbind() for a couple of corner cases. But, it altered the
>> errno for some other cases, for example, mbind() should return -EFAULT
>> when part or all of the memory range specified by nodemask and maxnode
>> points outside your accessible address space, or there was an unmapped
>> hole in the specified memory range specified by addr and len.
>>
>> Fixed this by preserving the errno returned by queue_pages_range().
>> And, the pagelist may be not empty even though queue_pages_range()
>> returns error, put the pages back to LRU since mbind_range() is not called
>> to really apply the policy so those pages should not be migrated, this
>> is also the old behavior before the problematic commit.
>Forgot fixes tag.
>
>Fixes: d883544515aa ("mm: mempolicy: make the behavior consistent when
>MPOL_MF_MOVE* and MPOL_MF_STRICT were specified")
>
Looks good to me.
Reviewed-by: Li Xinhai <lixinhai.lxh@gmail.com>
>> Reported-by: Li Xinhai <lixinhai.lxh@gmail.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: <stable@vger.kernel.org> v4.19 and v5.2+
>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>> ---
>> mm/mempolicy.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 4ae967b..e08c941 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -672,7 +672,9 @@ static int queue_pages_test_walk(unsigned long start, unsigned long end,
>> * 1 - there is unmovable page, but MPOL_MF_MOVE* & MPOL_MF_STRICT were
>> * specified.
>> * 0 - queue pages successfully or no misplaced page.
>> - * -EIO - there is misplaced page and only MPOL_MF_STRICT was specified.
>> + * errno - i.e. misplaced pages with MPOL_MF_STRICT specified (-EIO) or
>> + * memory range specified by nodemask and maxnode points outside
>> + * your accessible address space (-EFAULT)
>> */
>> static int
>> queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
>> @@ -1286,7 +1288,7 @@ static long do_mbind(unsigned long start, unsigned long len,
>> flags | MPOL_MF_INVERT, &pagelist);
>>
>> if (ret < 0) {
>> - err = -EIO;
>> + err = ret;
>> goto up_out;
>> }
>>
>> @@ -1305,10 +1307,12 @@ static long do_mbind(unsigned long start, unsigned long len,
>>
>> if ((ret > 0) || (nr_failed && (flags & MPOL_MF_STRICT)))
>> err = -EIO;
>> - } else
>> - putback_movable_pages(&pagelist);
>> -
>> + } else {
>> up_out:
>> + if (!list_empty(&pagelist))
>> + putback_movable_pages(&pagelist);
>> + }
>> +
>> up_write(&mm->mmap_sem);
>> mpol_out:
>> mpol_put(new);
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
2019-10-30 18:14 ` Yang Shi
2019-10-31 1:53 ` Li Xinhai
@ 2019-10-31 4:31 ` Andrew Morton
2019-10-31 5:28 ` Li Xinhai
2019-10-31 15:47 ` Yang Shi
1 sibling, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2019-10-31 4:31 UTC (permalink / raw)
To: Yang Shi
Cc: lixinhai.lxh, vbabka, mhocko, mgorman, stable, linux-mm,
linux-kernel, Li Xinhai
On Wed, 30 Oct 2019 11:14:58 -0700 Yang Shi <yang.shi@linux.alibaba.com> wrote:
> On 10/30/19 9:58 AM, Yang Shi wrote:
> > The commit d883544515aa ("mm: mempolicy: make the behavior consistent
> > when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
> > value of mbind() for a couple of corner cases. But, it altered the
> > errno for some other cases, for example, mbind() should return -EFAULT
> > when part or all of the memory range specified by nodemask and maxnode
> > points outside your accessible address space, or there was an unmapped
> > hole in the specified memory range specified by addr and len.
> >
> > Fixed this by preserving the errno returned by queue_pages_range().
> > And, the pagelist may be not empty even though queue_pages_range()
> > returns error, put the pages back to LRU since mbind_range() is not called
> > to really apply the policy so those pages should not be migrated, this
> > is also the old behavior before the problematic commit.
> Forgot fixes tag.
>
> Fixes: d883544515aa ("mm: mempolicy: make the behavior consistent when
> MPOL_MF_MOVE* and MPOL_MF_STRICT were specified")
What's the relationship between this patch and
http://lkml.kernel.org/r/201910291756045288126@gmail.com?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
2019-10-31 4:31 ` Andrew Morton
@ 2019-10-31 5:28 ` Li Xinhai
2019-10-31 15:47 ` Yang Shi
1 sibling, 0 replies; 6+ messages in thread
From: Li Xinhai @ 2019-10-31 5:28 UTC (permalink / raw)
To: akpm, yang.shi
Cc: Vlastimil Babka, mhocko, mgorman, stable, linux-mm, linux-kernel
On 2019-10-31 at 12:31 Andrew Morton wrote:
>On Wed, 30 Oct 2019 11:14:58 -0700 Yang Shi <yang.shi@linux.alibaba.com> wrote:
>
>> On 10/30/19 9:58 AM, Yang Shi wrote:
>> > The commit d883544515aa ("mm: mempolicy: make the behavior consistent
>> > when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
>> > value of mbind() for a couple of corner cases. But, it altered the
>> > errno for some other cases, for example, mbind() should return -EFAULT
>> > when part or all of the memory range specified by nodemask and maxnode
>> > points outside your accessible address space, or there was an unmapped
>> > hole in the specified memory range specified by addr and len.
>> >
>> > Fixed this by preserving the errno returned by queue_pages_range().
>> > And, the pagelist may be not empty even though queue_pages_range()
>> > returns error, put the pages back to LRU since mbind_range() is not called
>> > to really apply the policy so those pages should not be migrated, this
>> > is also the old behavior before the problematic commit.
>> Forgot fixes tag.
>>
>> Fixes: d883544515aa ("mm: mempolicy: make the behavior consistent when
>> MPOL_MF_MOVE* and MPOL_MF_STRICT were specified")
>
>What's the relationship between this patch and
>http://lkml.kernel.org/r/201910291756045288126@gmail.com?
>
They are for different issues. I found that -EFAULT is hidden from user space by
d883544515aa when I was fixing the unmapped hole issue which is described in
your quoted link.
Now, it is fixed for by current commit in this mail thread. I will explain the other
one in its thead.
- Xinhai
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind
2019-10-31 4:31 ` Andrew Morton
2019-10-31 5:28 ` Li Xinhai
@ 2019-10-31 15:47 ` Yang Shi
1 sibling, 0 replies; 6+ messages in thread
From: Yang Shi @ 2019-10-31 15:47 UTC (permalink / raw)
To: Andrew Morton
Cc: lixinhai.lxh, vbabka, mhocko, mgorman, stable, linux-mm, linux-kernel
On 10/30/19 9:31 PM, Andrew Morton wrote:
> On Wed, 30 Oct 2019 11:14:58 -0700 Yang Shi <yang.shi@linux.alibaba.com> wrote:
>
>> On 10/30/19 9:58 AM, Yang Shi wrote:
>>> The commit d883544515aa ("mm: mempolicy: make the behavior consistent
>>> when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") fixed the return
>>> value of mbind() for a couple of corner cases. But, it altered the
>>> errno for some other cases, for example, mbind() should return -EFAULT
>>> when part or all of the memory range specified by nodemask and maxnode
>>> points outside your accessible address space, or there was an unmapped
>>> hole in the specified memory range specified by addr and len.
>>>
>>> Fixed this by preserving the errno returned by queue_pages_range().
>>> And, the pagelist may be not empty even though queue_pages_range()
>>> returns error, put the pages back to LRU since mbind_range() is not called
>>> to really apply the policy so those pages should not be migrated, this
>>> is also the old behavior before the problematic commit.
>> Forgot fixes tag.
>>
>> Fixes: d883544515aa ("mm: mempolicy: make the behavior consistent when
>> MPOL_MF_MOVE* and MPOL_MF_STRICT were specified")
> What's the relationship between this patch and
> http://lkml.kernel.org/r/201910291756045288126@gmail.com?
They are irrelevant. The commit d883544515aa ("mm: mempolicy: make the
behavior consistent
when MPOL_MF_MOVE* and MPOL_MF_STRICT were specified") override the
-EFAULT return value of queue_pages_range() by -EIO mistakenly and
missed putting non-empty pagelist back, this patch is aimed to fix the
two issues.
I think Li Xinhai found the return value override problem during
debugging his patch.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-31 15:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-30 16:58 [PATCH] mm: mempolicy: fix the wrong return value and potential pages leak of mbind Yang Shi
2019-10-30 18:14 ` Yang Shi
2019-10-31 1:53 ` Li Xinhai
2019-10-31 4:31 ` Andrew Morton
2019-10-31 5:28 ` Li Xinhai
2019-10-31 15:47 ` Yang Shi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.