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

On 10/14/19 11:12 AM, Michal Hocko wrote:

quoted

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.