Commit 7485d0d3758e8e6491a5c9468114e74dc050785d

Authored by KOSAKI Motohiro
Committed by Ingo Molnar
1 parent 7284ce6c9f

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>

Showing 1 changed file with 12 additions and 15 deletions Side-by-side Diff

... ... @@ -203,8 +203,6 @@
203 203 * @uaddr: virtual address of the futex
204 204 * @fshared: 0 for a PROCESS_PRIVATE futex, 1 for PROCESS_SHARED
205 205 * @key: address where result is stored.
206   - * @rw: mapping needs to be read/write (values: VERIFY_READ,
207   - * VERIFY_WRITE)
208 206 *
209 207 * Returns a negative error code or 0
210 208 * The key words are stored in *key on success.
... ... @@ -216,7 +214,7 @@
216 214 * lock_page() might sleep, the caller should not hold a spinlock.
217 215 */
218 216 static int
219   -get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key, int rw)
  217 +get_futex_key(u32 __user *uaddr, int fshared, union futex_key *key)
220 218 {
221 219 unsigned long address = (unsigned long)uaddr;
222 220 struct mm_struct *mm = current->mm;
... ... @@ -239,7 +237,7 @@
239 237 * but access_ok() should be faster than find_vma()
240 238 */
241 239 if (!fshared) {
242   - if (unlikely(!access_ok(rw, uaddr, sizeof(u32))))
  240 + if (unlikely(!access_ok(VERIFY_WRITE, uaddr, sizeof(u32))))
243 241 return -EFAULT;
244 242 key->private.mm = mm;
245 243 key->private.address = address;
... ... @@ -248,7 +246,7 @@
248 246 }
249 247  
250 248 again:
251   - err = get_user_pages_fast(address, 1, rw == VERIFY_WRITE, &page);
  249 + err = get_user_pages_fast(address, 1, 1, &page);
252 250 if (err < 0)
253 251 return err;
254 252  
... ... @@ -867,7 +865,7 @@
867 865 if (!bitset)
868 866 return -EINVAL;
869 867  
870   - ret = get_futex_key(uaddr, fshared, &key, VERIFY_READ);
  868 + ret = get_futex_key(uaddr, fshared, &key);
871 869 if (unlikely(ret != 0))
872 870 goto out;
873 871  
874 872  
... ... @@ -913,10 +911,10 @@
913 911 int ret, op_ret;
914 912  
915 913 retry:
916   - ret = get_futex_key(uaddr1, fshared, &key1, VERIFY_READ);
  914 + ret = get_futex_key(uaddr1, fshared, &key1);
917 915 if (unlikely(ret != 0))
918 916 goto out;
919   - ret = get_futex_key(uaddr2, fshared, &key2, VERIFY_WRITE);
  917 + ret = get_futex_key(uaddr2, fshared, &key2);
920 918 if (unlikely(ret != 0))
921 919 goto out_put_key1;
922 920  
923 921  
... ... @@ -1175,11 +1173,10 @@
1175 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 1177 if (unlikely(ret != 0))
1180 1178 goto out;
1181   - ret = get_futex_key(uaddr2, fshared, &key2,
1182   - requeue_pi ? VERIFY_WRITE : VERIFY_READ);
  1179 + ret = get_futex_key(uaddr2, fshared, &key2);
1183 1180 if (unlikely(ret != 0))
1184 1181 goto out_put_key1;
1185 1182  
... ... @@ -1738,7 +1735,7 @@
1738 1735 */
1739 1736 retry:
1740 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 1739 if (unlikely(ret != 0))
1743 1740 return ret;
1744 1741  
... ... @@ -1904,7 +1901,7 @@
1904 1901 q.requeue_pi_key = NULL;
1905 1902 retry:
1906 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 1905 if (unlikely(ret != 0))
1909 1906 goto out;
1910 1907  
... ... @@ -2023,7 +2020,7 @@
2023 2020 if ((uval & FUTEX_TID_MASK) != task_pid_vnr(current))
2024 2021 return -EPERM;
2025 2022  
2026   - ret = get_futex_key(uaddr, fshared, &key, VERIFY_WRITE);
  2023 + ret = get_futex_key(uaddr, fshared, &key);
2027 2024 if (unlikely(ret != 0))
2028 2025 goto out;
2029 2026  
... ... @@ -2215,7 +2212,7 @@
2215 2212 rt_waiter.task = NULL;
2216 2213  
2217 2214 key2 = FUTEX_KEY_INIT;
2218   - ret = get_futex_key(uaddr2, fshared, &key2, VERIFY_WRITE);
  2215 + ret = get_futex_key(uaddr2, fshared, &key2);
2219 2216 if (unlikely(ret != 0))
2220 2217 goto out;
2221 2218