linux-kernel - Re: [PATCH] mm: mempolicy: fix the absence of the last bit of nodemask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 14 Oct 2019 11:35:46 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     Michal Hocko <mhocko@...nel.org>, Pan Zhang <zhangpan26@...wei.com>
Cc:     akpm@...ux-foundation.org, rientjes@...gle.com, jgg@...pe.ca,
        aarcange@...hat.com, yang.shi@...ux.alibaba.com,
        zhongjiang@...wei.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, Cristopher Lameter <cl@...ux.com>,
        Linux API <linux-api@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>
Subject: Re: [PATCH] mm: mempolicy: fix the absence of the last bit of
 nodemask

On 10/14/19 11:12 AM, Michal Hocko wrote:
>> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
>> index 4ae967b..a23509f 100644
>> --- a/mm/mempolicy.c
>> +++ b/mm/mempolicy.c
>> @@ -1328,9 +1328,11 @@ 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)
>> +	/*
>> +	 * If the user specified only one node, no need to set nodemask
>> +	 */
>> +	if (maxnode - 1 == 0 || !nmask)
>>  		return 0;
>>  	if (maxnode > PAGE_SIZE*BITS_PER_BYTE)
>>  		return -EINVAL;
> 
> I am afraid this is a wrong fix. It is really hard to grasp the code but my
> understanding is that the caller is supposed to provide maxnode larger
> than than the nodemask. So if you want 2 nodes then maxnode should be 3.
> Have a look at the libnuma (which is a reference implementation)
> 
> static void setpol(int policy, struct bitmask *bmp)
> {
> 	if (set_mempolicy(policy, bmp->maskp, bmp->size + 1) < 0)
> 		numa_error("set_mempolicy");
> }
> 
> The semantic is quite awkward but it is that way for years.

Yes, unfortunately. Too late to change. We could just update the
manpages at this point.

get_mempolicy(2) says:
 maxnode specifies the number of node IDs that can be stored into
nodemask—that is, the maximum node ID plus one.

- Since node ID starts with 0, it should be actually "plus two".

set_mempolicy(2) says:
 nodemask  points to a bit mask of node IDs that contains up to maxnode
bits.

- should be also clarified.