[PATCH] mm: mempolicy: Fix wrong use of maxnode in mempolicy API

[PATCH] mm: mempolicy: Fix wrong use of maxnode in mempolicy API

[Li Xinhai]

The maxnode used by mbind(), set_mempolicy() and migrate_pages() is not
correctly handled in get_nodes(), where --maxnode cause the calculation
  endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1;
ignores the highest node ID bit. Then, at
  nodes_addr(*nodes)[nlongs-1] &= endmask;
the highest node ID bit is cleared. Finally, cause mpol_new() think user
does not pass in any node ID, and return EINVAL.

The results are:
- Application receives EINVAL when only the highest node ID bit is set.
- Application recevies no error when other bits set together with the
  highest node ID bit, but that highest ID is ignored.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Hugh Dickins <hughd@google.com>
Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
---
 mm/mempolicy.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 4ae967b..2bdc365 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1328,7 +1328,6 @@ static int get_nodes(nodemask_t *nodes, const unsigned long __user *nmask,
 	unsigned long nlongs;
 	unsigned long endmask;
 
-	--maxnode;
 	nodes_clear(*nodes);
 	if (maxnode == 0 || !nmask)
 		return 0;
-- 
1.8.3.1

Re: [PATCH] mm: mempolicy: Fix wrong use of maxnode in mempolicy API

[Michal Hocko]

On Fri 08-11-19 23:49:29, Li Xinhai wrote:
> The maxnode used by mbind(), set_mempolicy() and migrate_pages() is not
> correctly handled in get_nodes(), where --maxnode cause the calculation
>   endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1;
> ignores the highest node ID bit. Then, at
>   nodes_addr(*nodes)[nlongs-1] &= endmask;
> the highest node ID bit is cleared. Finally, cause mpol_new() think user
> does not pass in any node ID, and return EINVAL.
> 
> The results are:
> - Application receives EINVAL when only the highest node ID bit is set.
> - Application recevies no error when other bits set together with the
>   highest node ID bit, but that highest ID is ignored.

A similar patch has been posted [1] and rejected [2].

[1] http://lkml.kernel.org/r/1570882789-20579-1-git-send-email-zhangpan26@huawei.com
[2] http://lkml.kernel.org/r/20191014091243.GD317@dhcp22.suse.cz

> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Hugh Dickins <hughd@google.com>
> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
> ---
>  mm/mempolicy.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 4ae967b..2bdc365 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1328,7 +1328,6 @@ static int get_nodes(nodemask_t *nodes, const unsigned long __user *nmask,
>  	unsigned long nlongs;
>  	unsigned long endmask;
>  
> -	--maxnode;
>  	nodes_clear(*nodes);
>  	if (maxnode == 0 || !nmask)
>  		return 0;
> -- 
> 1.8.3.1
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mempolicy: Fix wrong use of maxnode in mempolicy API

[lixinhai.lxh]

On 2019-11-09 at 00:04 Michal Hocko wrote:
>On Fri 08-11-19 23:49:29, Li Xinhai wrote:
>> The maxnode used by mbind(), set_mempolicy() and migrate_pages() is not
>> correctly handled in get_nodes(), where --maxnode cause the calculation
>>   endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1;
>> ignores the highest node ID bit. Then, at
>>   nodes_addr(*nodes)[nlongs-1] &= endmask;
>> the highest node ID bit is cleared. Finally, cause mpol_new() think user
>> does not pass in any node ID, and return EINVAL.
>>
>> The results are:
>> - Application receives EINVAL when only the highest node ID bit is set.
>> - Application recevies no error when other bits set together with the
>>   highest node ID bit, but that highest ID is ignored.
>
>A similar patch has been posted [1] and rejected [2].
>
>[1] http://lkml.kernel.org/r/1570882789-20579-1-git-send-email-zhangpan26@huawei.com
>[2] http://lkml.kernel.org/r/20191014091243.GD317@dhcp22.suse.cz
>
>
>--
>Michal Hocko
>SUSE Labs 

I just checked the code in 'history' log for curiosity, this --maxnode seems 
appeared since the very first commit. Mention about this special point of 
maxnode in manpage of these three APIs would be one solution, :).

Xinhai

Re: [PATCH] mm: mempolicy: Fix wrong use of maxnode in mempolicy API

[Michal Hocko]

On Sat 09-11-19 00:48:01, lixinhai.lxh@gmail.com wrote:
> On 2019-11-09 at 00:04 Michal Hocko wrote:
> >On Fri 08-11-19 23:49:29, Li Xinhai wrote:
> >> The maxnode used by mbind(), set_mempolicy() and migrate_pages() is not
> >> correctly handled in get_nodes(), where --maxnode cause the calculation
> >>   endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1;
> >> ignores the highest node ID bit. Then, at
> >>   nodes_addr(*nodes)[nlongs-1] &= endmask;
> >> the highest node ID bit is cleared. Finally, cause mpol_new() think user
> >> does not pass in any node ID, and return EINVAL.
> >>
> >> The results are:
> >> - Application receives EINVAL when only the highest node ID bit is set.
> >> - Application recevies no error when other bits set together with the
> >>   highest node ID bit, but that highest ID is ignored.
> >
> >A similar patch has been posted [1] and rejected [2].
> >
> >[1] http://lkml.kernel.org/r/1570882789-20579-1-git-send-email-zhangpan26@huawei.com
> >[2] http://lkml.kernel.org/r/20191014091243.GD317@dhcp22.suse.cz
> >
> >
> >--
> >Michal Hocko
> >SUSE Labs 
> 
> I just checked the code in 'history' log for curiosity, this --maxnode seems 
> appeared since the very first commit. Mention about this special point of 
> maxnode in manpage of these three APIs would be one solution, :).

Yes documenting this would be preferable.
-- 
Michal Hocko
SUSE Labs