All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm: fix do_mbind return value
@ 2015-03-05  2:20 Kazutomo Yoshii
  2015-03-05  6:53 ` David Rientjes
  0 siblings, 1 reply; 11+ messages in thread
From: Kazutomo Yoshii @ 2015-03-05  2:20 UTC (permalink / raw)
  To: linux-kernel

I noticed that numa_alloc_onnode() failed to allocate memory on a
specified node in v4.0-rc1. I added a code to check the return value
of walk_page_range() in queue_pages_range() so that do_mbind() only
returns an error number or zero.

Signed-off-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
---
 mm/mempolicy.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 4721046..ea79171 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -644,6 +644,7 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
 		.nmask = nodes,
 		.prev = NULL,
 	};
+	int err;
 	struct mm_walk queue_pages_walk = {
 		.hugetlb_entry = queue_pages_hugetlb,
 		.pmd_entry = queue_pages_pte_range,
@@ -652,7 +653,10 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
 		.private = &qp,
 	};
 -	return walk_page_range(start, end, &queue_pages_walk);
+	err = walk_page_range(start, end, &queue_pages_walk);
+	if (err < 0)
+		return err;
+	return 0;
 }
  /*
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: fix do_mbind return value
  2015-03-05  2:20 [PATCH] mm: fix do_mbind return value Kazutomo Yoshii
@ 2015-03-05  6:53 ` David Rientjes
  2015-03-05  7:44   ` Kazutomo Yoshii
  2015-03-05  8:02     ` Naoya Horiguchi
  0 siblings, 2 replies; 11+ messages in thread
From: David Rientjes @ 2015-03-05  6:53 UTC (permalink / raw)
  To: Kazutomo Yoshii; +Cc: Naoya Horiguchi, linux-kernel

On Wed, 4 Mar 2015, Kazutomo Yoshii wrote:

> I noticed that numa_alloc_onnode() failed to allocate memory on a
> specified node in v4.0-rc1. I added a code to check the return value
> of walk_page_range() in queue_pages_range() so that do_mbind() only
> returns an error number or zero.
> 

I assume this is libnuma-2.0.10?

> Signed-off-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> ---
>  mm/mempolicy.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4721046..ea79171 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -644,6 +644,7 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
>  		.nmask = nodes,
>  		.prev = NULL,
>  	};
> +	int err;
>  	struct mm_walk queue_pages_walk = {
>  		.hugetlb_entry = queue_pages_hugetlb,
>  		.pmd_entry = queue_pages_pte_range,
> @@ -652,7 +653,10 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
>  		.private = &qp,
>  	};
>  -	return walk_page_range(start, end, &queue_pages_walk);
> +	err = walk_page_range(start, end, &queue_pages_walk);
> +	if (err < 0)
> +		return err;
> +	return 0;
>  }
>   /*

I'm afraid I don't think this is the right fix, if walk_page_range() 
returns a positive value then it should be supplied by one of the 
callbacks in the struct mm_walk, which none of these happen to do.  I 
think this may be a problem with commit 6f4576e3687b ("mempolicy: apply 
page table walker on queue_pages_range()"), so let's add Naoya to the 
thread.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: fix do_mbind return value
  2015-03-05  6:53 ` David Rientjes
@ 2015-03-05  7:44   ` Kazutomo Yoshii
  2015-03-05  8:02     ` Naoya Horiguchi
  1 sibling, 0 replies; 11+ messages in thread
From: Kazutomo Yoshii @ 2015-03-05  7:44 UTC (permalink / raw)
  To: David Rientjes; +Cc: Naoya Horiguchi, linux-kernel

On 03/05/2015 12:53 AM, David Rientjes wrote:
> On Wed, 4 Mar 2015, Kazutomo Yoshii wrote:
>
>> I noticed that numa_alloc_onnode() failed to allocate memory on a
>> specified node in v4.0-rc1. I added a code to check the return value
>> of walk_page_range() in queue_pages_range() so that do_mbind() only
>> returns an error number or zero.
>>
> I assume this is libnuma-2.0.10?
I used libnuma-2.0.9.  Here is a strace output related to 
numa_alloc_onnode()

mmap(NULL, 4194304, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0) = 0x7fe9b8334000
mbind(0x7fe9b8334000, 4194304, MPOL_BIND, 0x1b43bf0, 1025, 0) = 1

I believe mbind() returning a positive number is just a wrong behavior.
A tricky part is that  libnuma only checks a negative error,
so numa_alloc_onnode() itself didn't fail.   I noticed this first when I 
was checking
memory placement using the proc pagemap.

>> Signed-off-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
>> ---
>>   mm/mempolicy.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 4721046..ea79171 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -644,6 +644,7 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
>>   		.nmask = nodes,
>>   		.prev = NULL,
>>   	};
>> +	int err;
>>   	struct mm_walk queue_pages_walk = {
>>   		.hugetlb_entry = queue_pages_hugetlb,
>>   		.pmd_entry = queue_pages_pte_range,
>> @@ -652,7 +653,10 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
>>   		.private = &qp,
>>   	};
>>   -	return walk_page_range(start, end, &queue_pages_walk);
>> +	err = walk_page_range(start, end, &queue_pages_walk);
>> +	if (err < 0)
>> +		return err;
>> +	return 0;
>>   }
>>    /*
> I'm afraid I don't think this is the right fix, if walk_page_range()
> returns a positive value then it should be supplied by one of the
> callbacks in the struct mm_walk, which none of these happen to do.  I
> think this may be a problem with commit 6f4576e3687b ("mempolicy: apply
> page table walker on queue_pages_range()"), so let's add Naoya to the
> thread.
Thank you for the pointer!
I think queue_pages_test_walk() returns 1.
My fix may not be in a right place but someone needs to fix this.

- kaz



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
  2015-03-05  6:53 ` David Rientjes
@ 2015-03-05  8:02     ` Naoya Horiguchi
  2015-03-05  8:02     ` Naoya Horiguchi
  1 sibling, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:02 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

# CCed Andrew and linux-mm

On Wed, Mar 04, 2015 at 10:53:27PM -0800, David Rientjes wrote:
> On Wed, 4 Mar 2015, Kazutomo Yoshii wrote:
> 
> > I noticed that numa_alloc_onnode() failed to allocate memory on a
> > specified node in v4.0-rc1. I added a code to check the return value
> > of walk_page_range() in queue_pages_range() so that do_mbind() only
> > returns an error number or zero.
> > 
> 
> I assume this is libnuma-2.0.10?
> 
> > Signed-off-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> > ---
> >  mm/mempolicy.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 4721046..ea79171 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -644,6 +644,7 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
> >  		.nmask = nodes,
> >  		.prev = NULL,
> >  	};
> > +	int err;
> >  	struct mm_walk queue_pages_walk = {
> >  		.hugetlb_entry = queue_pages_hugetlb,
> >  		.pmd_entry = queue_pages_pte_range,
> > @@ -652,7 +653,10 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
> >  		.private = &qp,
> >  	};
> >  -	return walk_page_range(start, end, &queue_pages_walk);
> > +	err = walk_page_range(start, end, &queue_pages_walk);
> > +	if (err < 0)
> > +		return err;
> > +	return 0;
> >  }
> >   /*
> 
> I'm afraid I don't think this is the right fix, if walk_page_range() 
> returns a positive value then it should be supplied by one of the 
> callbacks in the struct mm_walk, which none of these happen to do.  I 
> think this may be a problem with commit 6f4576e3687b ("mempolicy: apply 
> page table walker on queue_pages_range()"), so let's add Naoya to the 
> thread.

Thank you for reporting/forwarding, Yoshii-san and David.

This bug is in the pagewalk's common path, and the following patch should
fix it.

Thanks,
Naoya Horiguchi
---
>From 107fa3fb256bddff40a882c90af717af9863aed7 Mon Sep 17 00:00:00 2001
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Thu, 5 Mar 2015 16:37:37 +0900
Subject: [PATCH] mm: pagewalk: prevent positive return value of
 walk_page_test() from being passed to callers

walk_page_test() is purely pagewalk's internal stuff, and its positive return
values are not intended to be passed to the callers of pagewalk. However, in
the current code if the last vma in the do-while loop in walk_page_range()
happens to return a positive value, it leaks outside walk_page_range().
So the user visible effect is invalid/unexpected return value (according to
the reporter, mbind() causes it.)

This patch fixes it simply by reinitializing the return value after checked.

Another exposed interface, walk_page_vma(), already returns 0 for such cases
so no problem.

Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")
Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
 mm/pagewalk.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/pagewalk.c b/mm/pagewalk.c
index 75c1f2878519..29f2f8b853ae 100644
--- a/mm/pagewalk.c
+++ b/mm/pagewalk.c
@@ -265,8 +265,15 @@ int walk_page_range(unsigned long start, unsigned long end,
 			vma = vma->vm_next;
 
 			err = walk_page_test(start, next, walk);
-			if (err > 0)
+			if (err > 0) {
+				/*
+				 * positive return values are purely for
+				 * controlling the pagewalk, so should never
+				 * be passed to the callers.
+				 */
+				err = 0;
 				continue;
+			}
 			if (err < 0)
 				break;
 		}
-- 
1.9.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
@ 2015-03-05  8:02     ` Naoya Horiguchi
  0 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:02 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

# CCed Andrew and linux-mm

On Wed, Mar 04, 2015 at 10:53:27PM -0800, David Rientjes wrote:
> On Wed, 4 Mar 2015, Kazutomo Yoshii wrote:
> 
> > I noticed that numa_alloc_onnode() failed to allocate memory on a
> > specified node in v4.0-rc1. I added a code to check the return value
> > of walk_page_range() in queue_pages_range() so that do_mbind() only
> > returns an error number or zero.
> > 
> 
> I assume this is libnuma-2.0.10?
> 
> > Signed-off-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> > ---
> >  mm/mempolicy.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index 4721046..ea79171 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -644,6 +644,7 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
> >  		.nmask = nodes,
> >  		.prev = NULL,
> >  	};
> > +	int err;
> >  	struct mm_walk queue_pages_walk = {
> >  		.hugetlb_entry = queue_pages_hugetlb,
> >  		.pmd_entry = queue_pages_pte_range,
> > @@ -652,7 +653,10 @@ queue_pages_range(struct mm_struct *mm, unsigned long start, unsigned long end,
> >  		.private = &qp,
> >  	};
> >  -	return walk_page_range(start, end, &queue_pages_walk);
> > +	err = walk_page_range(start, end, &queue_pages_walk);
> > +	if (err < 0)
> > +		return err;
> > +	return 0;
> >  }
> >   /*
> 
> I'm afraid I don't think this is the right fix, if walk_page_range() 
> returns a positive value then it should be supplied by one of the 
> callbacks in the struct mm_walk, which none of these happen to do.  I 
> think this may be a problem with commit 6f4576e3687b ("mempolicy: apply 
> page table walker on queue_pages_range()"), so let's add Naoya to the 
> thread.

Thank you for reporting/forwarding, Yoshii-san and David.

This bug is in the pagewalk's common path, and the following patch should
fix it.

Thanks,
Naoya Horiguchi
---

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
  2015-03-05  8:02     ` Naoya Horiguchi
@ 2015-03-05  8:09       ` Naoya Horiguchi
  -1 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:09 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2239 bytes --]

On Thu, Mar 05, 2015 at 08:02:27AM +0000, Horiguchi Naoya(堀口 直也) wrote:
...
> ---
> From 107fa3fb256bddff40a882c90af717af9863aed7 Mon Sep 17 00:00:00 2001
> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Date: Thu, 5 Mar 2015 16:37:37 +0900
> Subject: [PATCH] mm: pagewalk: prevent positive return value of
>  walk_page_test() from being passed to callers
> 
> walk_page_test() is purely pagewalk's internal stuff, and its positive return
> values are not intended to be passed to the callers of pagewalk. However, in
> the current code if the last vma in the do-while loop in walk_page_range()
> happens to return a positive value, it leaks outside walk_page_range().
> So the user visible effect is invalid/unexpected return value (according to
> the reporter, mbind() causes it.)
> 
> This patch fixes it simply by reinitializing the return value after checked.
> 
> Another exposed interface, walk_page_vma(), already returns 0 for such cases
> so no problem.
> 
> Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")

This is not a right tag. To be precise, the bug was introduced by commit
fafaa4264eba ("pagewalk: improve vma handling"), so

  Fixes fafaa4264eba ("pagewalk: improve vma handling")

is right.

Thanks,
Naoya Horiguchi

> Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
>  mm/pagewalk.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index 75c1f2878519..29f2f8b853ae 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -265,8 +265,15 @@ int walk_page_range(unsigned long start, unsigned long end,
>  			vma = vma->vm_next;
>  
>  			err = walk_page_test(start, next, walk);
> -			if (err > 0)
> +			if (err > 0) {
> +				/*
> +				 * positive return values are purely for
> +				 * controlling the pagewalk, so should never
> +				 * be passed to the callers.
> +				 */
> +				err = 0;
>  				continue;
> +			}
>  			if (err < 0)
>  				break;
>  		}
> -- 
> 1.9.3
> ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
@ 2015-03-05  8:09       ` Naoya Horiguchi
  0 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:09 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

On Thu, Mar 05, 2015 at 08:02:27AM +0000, Horiguchi Naoya(堀口 直也) wrote:
...
> ---
> From 107fa3fb256bddff40a882c90af717af9863aed7 Mon Sep 17 00:00:00 2001
> From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> Date: Thu, 5 Mar 2015 16:37:37 +0900
> Subject: [PATCH] mm: pagewalk: prevent positive return value of
>  walk_page_test() from being passed to callers
> 
> walk_page_test() is purely pagewalk's internal stuff, and its positive return
> values are not intended to be passed to the callers of pagewalk. However, in
> the current code if the last vma in the do-while loop in walk_page_range()
> happens to return a positive value, it leaks outside walk_page_range().
> So the user visible effect is invalid/unexpected return value (according to
> the reporter, mbind() causes it.)
> 
> This patch fixes it simply by reinitializing the return value after checked.
> 
> Another exposed interface, walk_page_vma(), already returns 0 for such cases
> so no problem.
> 
> Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")

This is not a right tag. To be precise, the bug was introduced by commit
fafaa4264eba ("pagewalk: improve vma handling"), so

  Fixes fafaa4264eba ("pagewalk: improve vma handling")

is right.

Thanks,
Naoya Horiguchi

> Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
>  mm/pagewalk.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/pagewalk.c b/mm/pagewalk.c
> index 75c1f2878519..29f2f8b853ae 100644
> --- a/mm/pagewalk.c
> +++ b/mm/pagewalk.c
> @@ -265,8 +265,15 @@ int walk_page_range(unsigned long start, unsigned long end,
>  			vma = vma->vm_next;
>  
>  			err = walk_page_test(start, next, walk);
> -			if (err > 0)
> +			if (err > 0) {
> +				/*
> +				 * positive return values are purely for
> +				 * controlling the pagewalk, so should never
> +				 * be passed to the callers.
> +				 */
> +				err = 0;
>  				continue;
> +			}
>  			if (err < 0)
>  				break;
>  		}
> -- 
> 1.9.3
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
  2015-03-05  8:09       ` Naoya Horiguchi
@ 2015-03-05  8:27         ` Naoya Horiguchi
  -1 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:27 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

> > From 107fa3fb256bddff40a882c90af717af9863aed7 Mon Sep 17 00:00:00 2001
> > From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> > Date: Thu, 5 Mar 2015 16:37:37 +0900
> > Subject: [PATCH] mm: pagewalk: prevent positive return value of
> >  walk_page_test() from being passed to callers
> > 
> > walk_page_test() is purely pagewalk's internal stuff, and its positive return
> > values are not intended to be passed to the callers of pagewalk. However, in
> > the current code if the last vma in the do-while loop in walk_page_range()
> > happens to return a positive value, it leaks outside walk_page_range().
> > So the user visible effect is invalid/unexpected return value (according to
> > the reporter, mbind() causes it.)
> > 
> > This patch fixes it simply by reinitializing the return value after checked.
> > 
> > Another exposed interface, walk_page_vma(), already returns 0 for such cases
> > so no problem.
> > 
> > Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")
> 
> This is not a right tag. To be precise, the bug was introduced by commit
> fafaa4264eba ("pagewalk: improve vma handling"), so
> 
>   Fixes fafaa4264eba ("pagewalk: improve vma handling")
> 
> is right.
> 
> Thanks,
> Naoya Horiguchi
> 
> > Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>

Ah, I might be a kind of rude, the original idea was posted by Yoshii-san,
and I changed it, so I may as well add his Signed-off-by (additional to
Reported-by) ?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
@ 2015-03-05  8:27         ` Naoya Horiguchi
  0 siblings, 0 replies; 11+ messages in thread
From: Naoya Horiguchi @ 2015-03-05  8:27 UTC (permalink / raw)
  To: David Rientjes; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

> > From 107fa3fb256bddff40a882c90af717af9863aed7 Mon Sep 17 00:00:00 2001
> > From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> > Date: Thu, 5 Mar 2015 16:37:37 +0900
> > Subject: [PATCH] mm: pagewalk: prevent positive return value of
> >  walk_page_test() from being passed to callers
> > 
> > walk_page_test() is purely pagewalk's internal stuff, and its positive return
> > values are not intended to be passed to the callers of pagewalk. However, in
> > the current code if the last vma in the do-while loop in walk_page_range()
> > happens to return a positive value, it leaks outside walk_page_range().
> > So the user visible effect is invalid/unexpected return value (according to
> > the reporter, mbind() causes it.)
> > 
> > This patch fixes it simply by reinitializing the return value after checked.
> > 
> > Another exposed interface, walk_page_vma(), already returns 0 for such cases
> > so no problem.
> > 
> > Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")
> 
> This is not a right tag. To be precise, the bug was introduced by commit
> fafaa4264eba ("pagewalk: improve vma handling"), so
> 
>   Fixes fafaa4264eba ("pagewalk: improve vma handling")
> 
> is right.
> 
> Thanks,
> Naoya Horiguchi
> 
> > Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>

Ah, I might be a kind of rude, the original idea was posted by Yoshii-san,
and I changed it, so I may as well add his Signed-off-by (additional to
Reported-by) ?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
  2015-03-05  8:02     ` Naoya Horiguchi
@ 2015-03-05 20:34       ` David Rientjes
  -1 siblings, 0 replies; 11+ messages in thread
From: David Rientjes @ 2015-03-05 20:34 UTC (permalink / raw)
  To: Naoya Horiguchi; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

On Thu, 5 Mar 2015, Naoya Horiguchi wrote:

> walk_page_test() is purely pagewalk's internal stuff, and its positive return
> values are not intended to be passed to the callers of pagewalk. However, in
> the current code if the last vma in the do-while loop in walk_page_range()
> happens to return a positive value, it leaks outside walk_page_range().
> So the user visible effect is invalid/unexpected return value (according to
> the reporter, mbind() causes it.)
> 
> This patch fixes it simply by reinitializing the return value after checked.
> 
> Another exposed interface, walk_page_vma(), already returns 0 for such cases
> so no problem.
> 
> Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")
> Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

Acked-by: David Rientjes <rientjes@google.com>

This is exactly what I had in mind, thanks for fixing it up so fast!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value)
@ 2015-03-05 20:34       ` David Rientjes
  0 siblings, 0 replies; 11+ messages in thread
From: David Rientjes @ 2015-03-05 20:34 UTC (permalink / raw)
  To: Naoya Horiguchi; +Cc: Kazutomo Yoshii, linux-kernel, Andrew Morton, linux-mm

On Thu, 5 Mar 2015, Naoya Horiguchi wrote:

> walk_page_test() is purely pagewalk's internal stuff, and its positive return
> values are not intended to be passed to the callers of pagewalk. However, in
> the current code if the last vma in the do-while loop in walk_page_range()
> happens to return a positive value, it leaks outside walk_page_range().
> So the user visible effect is invalid/unexpected return value (according to
> the reporter, mbind() causes it.)
> 
> This patch fixes it simply by reinitializing the return value after checked.
> 
> Another exposed interface, walk_page_vma(), already returns 0 for such cases
> so no problem.
> 
> Fixes: 6f4576e3687b ("mempolicy: apply page table walker on queue_pages_range()")
> Reported-by: Kazutomo Yoshii <kazutomo.yoshii@gmail.com>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

Acked-by: David Rientjes <rientjes@google.com>

This is exactly what I had in mind, thanks for fixing it up so fast!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2015-03-05 20:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-05  2:20 [PATCH] mm: fix do_mbind return value Kazutomo Yoshii
2015-03-05  6:53 ` David Rientjes
2015-03-05  7:44   ` Kazutomo Yoshii
2015-03-05  8:02   ` [PATCH] mm: pagewalk: prevent positive return value of walk_page_test() from being passed to callers (Re: [PATCH] mm: fix do_mbind return value) Naoya Horiguchi
2015-03-05  8:02     ` Naoya Horiguchi
2015-03-05  8:09     ` Naoya Horiguchi
2015-03-05  8:09       ` Naoya Horiguchi
2015-03-05  8:27       ` Naoya Horiguchi
2015-03-05  8:27         ` Naoya Horiguchi
2015-03-05 20:34     ` David Rientjes
2015-03-05 20:34       ` David Rientjes

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.