[PATCH] off-by-one in get_mempolicy(2)

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

	get_mempolicy(2) and related syscalls have always passed
1 + number of bits in nodemask as maxnodes argument - see e.g.
copy_nodes_to_user() and get_nodes().  Or libnuma, for the userland
side -
static void getpol(int *oldpolicy, struct bitmask *bmp)
{
        if (get_mempolicy(oldpolicy, bmp->maskp, bmp->size + 1, 0, 0) < 0)
                numa_error("get_mempolicy");
}
and similar for other syscalls.  However, the check for insufficient
destination size in get_mempolicy(2) used to be
        if (nmask != NULL && maxnode < MAX_NUMNODES)
                return -EINVAL;
IOW, maxnode == MAX_NUMNODES (representing "MAX_NUMNODES - 1 bits")
had been accepted.  The reason why that hadn't messed libnuma
logics used to determine the required bitmap size is that
MAX_NUMNODES is always a power of 2 and the loop in libnuma
is
                nodemask_sz = 16;
                do {
                        nodemask_sz <<= 1;
                        mask = realloc(mask, nodemask_sz / 8);
                        if (!mask)
                                return;
                } while (get_mempolicy(&pol, mask, nodemask_sz + 1, 0, 0) < 0 && errno == EINVAL &&
                                nodemask_sz < 4096*8);
I.e. it's been passing 33, 65, 127, etc. until it got it large enough.
That sidesteps the boundary case - we never try to pass exactly
MAX_NUMNODES there.

However, that has changed recently, when get_mempolicy() switched
to 
        if (nmask != NULL && maxnode < nr_node_ids)
                return -EINVAL;
_That_ can trigger.  Consider a box with nr_node_ids == 65.
The first call in libnuma:set_nodemask_size() loop will
pass 33 and fail, then we'll raise nodemask_sz to 64,
allocate a 64bit mask and call get_mempolicy(&pol, mask, 65, 0, 0),
which will succeed.  OK, so we decide to use 64bit bitmaps, and
subsequent getpol() will be passing 65 to get_mempolicy(2).  Which
is not a good idea, since kernel-side we'll get
	copy_nodes_to_user(nmask, 65, &nodes)
And that will copy only 8 bytes out of kernel-side bitmap with
65 bits in it...

IOW, that check always should had been <=, not <; it didn't matter
until commit 050c17f239fd ("numa: change get_mempolicy() to use
nr_node_ids instead of MAX_NUMNODES") this year.  The fix is trivial
- we need to make that check consistent with the code that does
actual copyin/copyout.

Fixes: 050c17f239fd ("numa: change get_mempolicy() to use nr_node_ids instead of MAX_NUMNODES")
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
---
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 4ae967bcf954..e184df7633b0 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -1561,7 +1561,7 @@ static int kernel_get_mempolicy(int __user *policy,
 
 	addr = untagged_addr(addr);
 
-	if (nmask != NULL && maxnode < nr_node_ids)
+	if (nmask != NULL && maxnode <= nr_node_ids)
 		return -EINVAL;
 
 	err = do_get_mempolicy(&pval, &nodes, addr, flags);