Hide Forgot
From Bugzilla Helper: User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.10) Gecko/20050719 Red Hat/1.0.6-1.4.1 Firefox/1.0.6 Description of problem: Note: since this is easly reproduced by an untrusted user I am flagging this as a security issue. Passing a 0 bitmask to set_mempolicy will cause a panic later on when a page fault is triggered. This panic can be triggered by ANY USER. I have only tested this on ia64 however I do not believe this is platform specific. I do expect however that it will only affect platforms where CONFIG_NUMA is set. The following code will reproduce the panic on ia64 when run as any user. #include <asm/unistd.h> main(){ syscall(__NR_set_mempolicy,3,0,0); write(1,10,8192); } This will cause the panic: kernel BUG at mm/mempolicy.c:665! a.out[3488]: bugcheck! 0 [1] The set_memplicy call causes current->il_next to be set to 256 which later on triggeres the panic from the BUG_ON line at mempolicy.c:665. This appears to be fixed upstream however the sys_set_mempolicy code has not changed. There are some changes to get_nodes that I am guessing fixes this issue. There are 2 calls added to get_nodes: cpuset_update_current_mems_allowed(); cpuset_restrict_to_mems_allowed(nodes); Which are both new routines added to kernel/cpuset.c. I have not investigated how these work but this is the only noticiable change that I have found to explain why I do not get the panic on 2.6.14.3. Version-Release number of selected component (if applicable): kernel-2.6.9-25.EL How reproducible: Always Steps to Reproduce: 1. cc mpol_panic.c 2. ./a.out 3. Actual Results: panic! Expected Results: no panic! Additional info:
Based on that comment this is likely to have no longer been an issue after 20050310, based on this update: http://linux.bkbits.net:8080/linux-2.6/cset@42307e9fp8ihEMrfaoPMp_agDevQNA Therefore 2.6 prior to 2.6.11
I think just changing get_nodes() so that it obeys the set_mempolicy() man page is the right thing to do here: It says all policies except MPOL_DEFAULT(3) require to specify the nodes they apply to in the nodemask parameter. This patch to get_nodes() will do just that which causes the set_mempolicy system call to return -EINVAL when the invalid parameters are passed in: ---------------------------------------------------------------- --- linux-2.6.9/mm/mempolicy.c.orig +++ linux-2.6.9/mm/mempolicy.c @@ -134,6 +134,8 @@ static int get_nodes(unsigned long *node --maxnode; bitmap_zero(nodes, MAX_NUMNODES); + if ((mode != MPOL_DEFAULT) && !nmask) + return -EINVAL; if (maxnode == 0 || !nmask) return 0; ----------------------------------------------------------------
I am still able to reproduce the panic however with different arguments passed to set_mempolicy. The resulting panic is identical to above, caused by this code in interleave_nodes() struct task_struct *me = current; nid = me->il_next; BUG_ON(nid >= MAX_NUMNODES); In all the dumps I have seen il_next is 256. This is set back in sys_set_mempolicy: if (new && new->policy == MPOL_INTERLEAVE) current->il_next = find_first_bit(new->v.nodes, MAX_NUMNODES); Perhaps, the best fix is something like: if (new && new->policy == MPOL_INTERLEAVE) { b = find_first_bit(new->v.nodes, MAX_NUMNODES); if (b >= MAX_NUMNODES) return -EINVAL; current->il_next = b; } Although I am sure there is a cleaner way. It is starting to look like this might be ia64 specific since find_first_bit is arch specific. Has anybody tried this elsewhere (note requires CONFIG_NUMA). I don't have access to any other systems to try this on.
FYI, here is a reproducer for this latest panic. Different mask and a maxnode of 1. #include <asm/unistd.h> main(){ syscall(__NR_set_mempolicy,3,0xffffffffafee4357,1); write(1,10,8192); } I tried other values for maxnode (0, 2, 3 and 4) and it looks like it is just "1" that causes it to panic with this imporper mask. Note that the mask was just randomly generated by the test. There are likely other masks that hit this.
I think I figured out what is going wrong here with the new patch. It _almost_ fixes the problem however there is still a way for the mask to end up being set to something like 2 (or anything with bit 0 not set). In get_nodes if we pass a bitmask with bit 0 not set and a maxnodes of 1: nlongs = BITS_TO_LONGS(maxnode); if ((maxnode % BITS_PER_LONG) == 0) endmask = ~0UL; else endmask = (1UL << (maxnode % BITS_PER_LONG)) - 1; endmask is now 1 ... if (copy_from_user(nodes, nmask, nlongs*sizeof(unsigned long))) return -EFAULT; nodes[nlongs-1] &= endmask; nodes was '0b10' and endmask was '0b01' & them together and we have nodes = 0 and the same problem as before. So, the original fix will work as long as we move it down to the bottom of the function and check "nodes" instead of "nmask".
Or even more importantly I just realized that the check: if ((mode != MPOL_DEFAULT) && !nmask) return -EINVAL; Only checks if the user specified a null pointer and doesn't check if the data that nmask points to is valid. That is probably more likely, we are passing a pointer to a null mask.
Doug, this patch appears to fix this problem. Did you get a chance to tsst it yet? -------------------------------------------------------------------------------- --- linux-2.6.9/mm/mempolicy.c.orig +++ linux-2.6.9/mm/mempolicy.c @@ -131,9 +131,12 @@ static int get_nodes(unsigned long *node unsigned long k; unsigned long nlongs; unsigned long endmask; + int i, bits = 0; --maxnode; bitmap_zero(nodes, MAX_NUMNODES); + if ((mode != MPOL_DEFAULT) && !nmask) + return -EINVAL; if (maxnode == 0 || !nmask) return 0; @@ -165,6 +168,13 @@ static int get_nodes(unsigned long *node if (copy_from_user(nodes, nmask, nlongs*sizeof(unsigned long))) return -EFAULT; nodes[nlongs-1] &= endmask; + /* the nmask can not be zero */ + for (i=0; i<nlongs; i++) + bits += !nodes[i]; + if (bits) + return -EINVAL; + + return mpol_check_policy(mode, nodes); } @@ -401,7 +411,7 @@ asmlinkage long sys_set_mempolicy(int mo mpol_free(current->mempolicy); current->mempolicy = new; if (new && new->policy == MPOL_INTERLEAVE) - current->il_next = find_first_bit(new->v.nodes, MAX_NUMNODES); + current->il_next = (find_first_bit(new->v.nodes, MAX_NUMNODES) & MAX_NUMNODES-1); return 0; }
An advisory has been issued which should help the problem described in this bug report. This report is therefore being closed with a resolution of ERRATA. For more information on the solution and/or where to find the updated files, please follow the link below. You may reopen this bug report if the solution does not work for you. http://rhn.redhat.com/errata/RHSA-2006-0101.html