aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>2010-01-05 02:32:43 -0500
committerIngo Molnar <mingo@elte.hu>2010-01-13 03:17:36 -0500
commit7485d0d3758e8e6491a5c9468114e74dc050785d (patch)
tree073fb295550c96f075cbf968858bbab7e9fe5061
parent7284ce6c9f6153d1777df5f310c959724d1bd446 (diff)
futexes: Remove rw parameter from get_futex_key()
Currently, futexes have two problem: A) The current futex code doesn't handle private file mappings properly. get_futex_key() uses PageAnon() to distinguish file and anon, which can cause the following bad scenario: 1) thread-A call futex(private-mapping, FUTEX_WAIT), it sleeps on file mapping object. 2) thread-B writes a variable and it makes it cow. 3) thread-B calls futex(private-mapping, FUTEX_WAKE), it wakes up blocked thread on the anonymous page. (but it's nothing) B) Current futex code doesn't handle zero page properly. Read mode get_user_pages() can return zero page, but current futex code doesn't handle it at all. Then, zero page makes infinite loop internally. The solution is to use write mode get_user_page() always for page lookup. It prevents the lookup of both file page of private mappings and zero page. Performance concerns: Probaly very little, because glibc always initialize variables for futex before to call futex(). It means glibc users never see the overhead of this patch. Compatibility concerns: This patch has few compatibility issues. After this patch, FUTEX_WAIT require writable access to futex variables (read-only mappings makes EFAULT). But practically it's not a problem, glibc always initalizes variables for futexes explicitly - nobody uses read-only mappings. Reported-by: Hugh Dickins <hugh.dickins@tiscali.co.uk> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> Acked-by: Peter Zijlstra <peterz@infradead.org> Acked-by: Darren Hart <dvhltc@us.ibm.com> Cc: <stable@kernel.org> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Cc: Nick Piggin <npiggin@suse.de> Cc: Ulrich Drepper <drepper@gmail.com> LKML-Reference: <20100105162633.45A2.A69D9226@jp.fujitsu.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
-rw-r--r--kernel/futex.c27
1 files changed, 12 insertions, 15 deletions
diff --git a/kernel/futex.c b/kernel/futex.c
index 8e3c3ffe1b9a..d9b3a2228f9d 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -203,8 +203,6 @@ static void drop_futex_key_refs(union futex_key *key)
203 * @uaddr: virtual address of the futex 203 * @uaddr: virtual address of the futex
204 * @fshared: 0 for a PROCESS_PRIVATE futex, 1 for PROCESS_SHARED 204 * @fshared: 0 for a PROCESS_PRIVATE futex, 1 for PROCESS_SHARED
205 * @key: address where result is stored. 205 * @key: address where result is stored.
206 * @rw: mapping needs to be read/write (values: VERIFY_READ,
207 * VERIFY_WRITE)
208 * 206 *
209 * Returns a negative error code or 0 207 * Returns a negative error code or 0
210 * The key words are stored in *key on success. 208 * The key words are stored in *key on success.
@@ -216,7 +214,7 @@ static void drop_futex_key_refs(union futex_key *key)
216 * lock_page() might sleep, the caller should not hold a spinlock. 214 * lock_page() might sleep, the caller should not hold a spinlock.
217 */ 215 */
218static int 216static int
219get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw) 217get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key)
220{ 218{
221 unsigned long address = (unsigned long)uaddr; 219 unsigned long address = (unsigned long)uaddr;
222 struct mm_struct *mm = current->mm; 220 struct mm_struct *mm = current->mm;
@@ -239,7 +237,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
239 * but access_ok() should be faster than find_vma() 237 * but access_ok() should be faster than find_vma()
240 */ 238 */
241 if (!fshared) { 239 if (!fshared) {
242 if (unlikely(!access_ok(rw, uaddr, sizeof(u32)))) 240 if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
243 return -EFAULT; 241 return -EFAULT;
244 key->private.mm = mm; 242 key->private.mm = mm;
245 key->private.address = address; 243 key->private.address = address;
@@ -248,7 +246,7 @@ get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
248 } 246 }
249 247
250again: 248again:
251 err = get_user_pages_fast(address, 1, rw == VERIFY_WRITE, &page); 249 err = get_user_pages_fast(address, 1, 1, &page);
252 if (err < 0) 250 if (err < 0)
253 return err; 251 return err;
254 252
@@ -867,7 +865,7 @@ static int futex_wake(u32 __user *uaddr, int fshared, int nr_wake, u32 bitset)
867 if (!bitset) 865 if (!bitset)
868 return -EINVAL; 866 return -EINVAL;
869 867
870 ret = get_futex_key(uaddr, fshared, &key, VERIFY_READ); 868 ret = get_futex_key(uaddr, fshared, &key);
871 if (unlikely(ret != 0)) 869 if (unlikely(ret != 0))
872 goto out; 870 goto out;
873 871
@@ -913,10 +911,10 @@ futex_wake_op(u32 __user *uaddr1, int fshared, u32 __user *uaddr2,
913 int ret, op_ret; 911 int ret, op_ret;
914 912
915retry: 913retry:
916 ret = get_futex_key(uaddr1, fshared, &key1, VERIFY_READ); 914 ret = get_futex_key(uaddr1, fshared, &key1);
917 if (unlikely(ret != 0)) 915 if (unlikely(ret != 0))
918 goto out; 916 goto out;
919 ret = get_futex_key(uaddr2, fshared, &key2, VERIFY_WRITE); 917 ret = get_futex_key(uaddr2, fshared, &key2);
920 if (unlikely(ret != 0)) 918 if (unlikely(ret != 0))
921 goto out_put_key1; 919 goto out_put_key1;
922 920
@@ -1175,11 +1173,10 @@ retry:
1175 pi_state = NULL; 1173 pi_state = NULL;
1176 } 1174 }
1177 1175
1178 ret = get_futex_key(uaddr1, fshared, &key1, VERIFY_READ); 1176 ret = get_futex_key(uaddr1, fshared, &key1);
1179 if (unlikely(ret != 0)) 1177 if (unlikely(ret != 0))
1180 goto out; 1178 goto out;
1181 ret = get_futex_key(uaddr2, fshared, &key2, 1179 ret = get_futex_key(uaddr2, fshared, &key2);
1182 requeue_pi ? VERIFY_WRITE : VERIFY_READ);
1183 if (unlikely(ret != 0)) 1180 if (unlikely(ret != 0))
1184 goto out_put_key1; 1181 goto out_put_key1;
1185 1182
@@ -1738,7 +1735,7 @@ static int futex_wait_setup(u32 __user *uaddr, u32 val, int fshared,
1738 */ 1735 */
1739retry: 1736retry:
1740 q->key = FUTEX_KEY_INIT; 1737 q->key = FUTEX_KEY_INIT;
1741 ret = get_futex_key(uaddr, fshared, &q->key, VERIFY_READ); 1738 ret = get_futex_key(uaddr, fshared, &q->key);
1742 if (unlikely(ret != 0)) 1739 if (unlikely(ret != 0))
1743 return ret; 1740 return ret;
1744 1741
@@ -1904,7 +1901,7 @@ static int futex_lock_pi(u32 __user *uaddr, int fshared,
1904 q.requeue_pi_key = NULL; 1901 q.requeue_pi_key = NULL;
1905retry: 1902retry:
1906 q.key = FUTEX_KEY_INIT; 1903 q.key = FUTEX_KEY_INIT;
1907 ret = get_futex_key(uaddr, fshared, &q.key, VERIFY_WRITE); 1904 ret = get_futex_key(uaddr, fshared, &q.key);
1908 if (unlikely(ret != 0)) 1905 if (unlikely(ret != 0))
1909 goto out; 1906 goto out;
1910 1907
@@ -2023,7 +2020,7 @@ retry:
2023 if ((uval & FUTEX_TID_MASK) != task_pid_vnr(current)) 2020 if ((uval & FUTEX_TID_MASK) != task_pid_vnr(current))
2024 return -EPERM; 2021 return -EPERM;
2025 2022
2026 ret = get_futex_key(uaddr, fshared, &key, VERIFY_WRITE); 2023 ret = get_futex_key(uaddr, fshared, &key);
2027 if (unlikely(ret != 0)) 2024 if (unlikely(ret != 0))
2028 goto out; 2025 goto out;
2029 2026
@@ -2215,7 +2212,7 @@ static int futex_wait_requeue_pi(u32 __user *uaddr, int fshared,
2215 rt_waiter.task = NULL; 2212 rt_waiter.task = NULL;
2216 2213
2217 key2 = FUTEX_KEY_INIT; 2214 key2 = FUTEX_KEY_INIT;
2218 ret = get_futex_key(uaddr2, fshared, &key2, VERIFY_WRITE); 2215 ret = get_futex_key(uaddr2, fshared, &key2);
2219 if (unlikely(ret != 0)) 2216 if (unlikely(ret != 0))
2220 goto out; 2217 goto out;
2221 2218