From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============1940925121305421359==" MIME-Version: 1.0 From: kbuild test robot To: kbuild-all@lists.01.org Subject: Re: [RFC PATCH] futex: don't retry futex_wait() with stale uaddr/val after spurious wakeup Date: Tue, 12 Nov 2019 10:36:04 +0800 Message-ID: <201911121003.Fg76xJzD%lkp@intel.com> In-Reply-To: <9179dbc3505e1de99ee36b09b0a12995239d73c3.1573079868.git.jstancek@redhat.com> List-Id: --===============1940925121305421359== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Hi Jan, [FYI, it's a private test report for your RFC patch.] [auto build test WARNING on tip/locking/core] [cannot apply to v5.4-rc7 next-20191111] [if your patch is applied to the wrong git tree, please drop us a note to h= elp improve the system. BTW, we also suggest to use '--base' option to specify = the base tree in git format-patch, please see https://stackoverflow.com/a/37406= 982] url: https://github.com/0day-ci/linux/commits/Jan-Stancek/futex-don-t-re= try-futex_wait-with-stale-uaddr-val-after-spurious-wakeup/20191109-055627 base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git a0855d2= 4fc22d49cdc25664fb224caee16998683 reproduce: # apt-get install sparse # sparse version: v0.6.1-29-g781bc5d-dirty make ARCH=3Dx86_64 allmodconfig make C=3D1 CF=3D'-fdiagnostic-prefix -D__CHECK_ENDIAN__' If you fix the issue, kindly add following tag Reported-by: kbuild test robot sparse warnings: (new ones prefixed by >>) >> kernel/futex.c:2198:29: sparse: sparse: incorrect type in assignment (di= fferent address spaces) @@ expected unsigned int [usertype] *uaddr @@ = got unsigned int [nodeunsigned int [usertype] *uaddr @@ >> kernel/futex.c:2198:29: sparse: expected unsigned int [usertype] *uad= dr >> kernel/futex.c:2198:29: sparse: got unsigned int [noderef] [usertype]= *uaddr2 >> kernel/futex.c:2732:17: sparse: sparse: incorrect type in assignment (di= fferent address spaces) @@ expected unsigned int [usertype] *[assigned] = uaddr @@ got unsigneunsigned int [usertype] *[assigned] uaddr @@ >> kernel/futex.c:2732:17: sparse: expected unsigned int [usertype] *[as= signed] uaddr >> kernel/futex.c:2732:17: sparse: got unsigned int [noderef] [usertype]= *uaddr >> kernel/futex.c:2742:33: sparse: sparse: incorrect type in argument 1 (di= fferent address spaces) @@ expected unsigned int [noderef] [usertype] *uaddr @@ got d int [noderef] [usertype] *uaddr @@ >> kernel/futex.c:2742:33: sparse: expected unsigned int [noderef] [user= type] *uaddr >> kernel/futex.c:2742:33: sparse: got unsigned int [usertype] *[assigne= d] uaddr kernel/futex.c:1562:9: sparse: sparse: context imbalance in 'wake_futex_= pi' - unexpected unlock kernel/futex.c:1722:33: sparse: sparse: context imbalance in 'futex_wake= _op' - different lock contexts for basic block include/linux/kasan-checks.h:38:20: sparse: sparse: context imbalance in= 'futex_requeue' - different lock contexts for basic block kernel/futex.c:2503:9: sparse: sparse: context imbalance in 'fixup_pi_st= ate_owner' - unexpected unlock kernel/futex.c:2612:13: sparse: sparse: context imbalance in 'futex_wait= _queue_me' - unexpected unlock kernel/futex.c:2714:1: sparse: sparse: context imbalance in 'futex_wait_= setup' - different lock contexts for basic block kernel/futex.c:2995:12: sparse: sparse: context imbalance in 'futex_unlo= ck_pi' - different lock contexts for basic block kernel/futex.c:3273:29: sparse: sparse: context imbalance in 'futex_wait= _requeue_pi' - unexpected unlock vim +2198 kernel/futex.c 1916 = 1917 /** 1918 * futex_requeue() - Requeue waiters from uaddr1 to uaddr2 1919 * @uaddr1: source futex user address 1920 * @flags: futex flags (FLAGS_SHARED, etc.) 1921 * @uaddr2: target futex user address 1922 * @nr_wake: number of waiters to wake (must be 1 for requeue_pi) 1923 * @nr_requeue: number of waiters to requeue (0-INT_MAX) 1924 * @cmpval: @uaddr1 expected value (or %NULL) 1925 * @requeue_pi: if we are attempting to requeue from a non-pi futex = to a 1926 * pi futex (pi to pi requeue is not supported) 1927 * 1928 * Requeue waiters on uaddr1 to uaddr2. In the requeue_pi case, try = to acquire 1929 * uaddr2 atomically on behalf of the top waiter. 1930 * 1931 * Return: 1932 * - >=3D0 - on success, the number of tasks requeued or woken; 1933 * - <0 - on error 1934 */ 1935 static int futex_requeue(u32 __user *uaddr1, unsigned int flags, 1936 u32 __user *uaddr2, int nr_wake, int nr_requeue, 1937 u32 *cmpval, int requeue_pi) 1938 { 1939 union futex_key key1 =3D FUTEX_KEY_INIT, key2 =3D FUTEX_KEY_INIT; 1940 int drop_count =3D 0, task_count =3D 0, ret; 1941 struct futex_pi_state *pi_state =3D NULL; 1942 struct futex_hash_bucket *hb1, *hb2; 1943 struct futex_q *this, *next; 1944 DEFINE_WAKE_Q(wake_q); 1945 u32 curval, targetval; 1946 = 1947 if (nr_wake < 0 || nr_requeue < 0) 1948 return -EINVAL; 1949 = 1950 /* 1951 * When PI not supported: return -ENOSYS if requeue_pi is true, 1952 * consequently the compiler knows requeue_pi is always false past 1953 * this point which will optimize away all the conditional code 1954 * further down. 1955 */ 1956 if (!IS_ENABLED(CONFIG_FUTEX_PI) && requeue_pi) 1957 return -ENOSYS; 1958 = 1959 if (requeue_pi) { 1960 /* 1961 * Requeue PI only works on two distinct uaddrs. This 1962 * check is only valid for private futexes. See below. 1963 */ 1964 if (uaddr1 =3D=3D uaddr2) 1965 return -EINVAL; 1966 = 1967 /* 1968 * requeue_pi requires a pi_state, try to allocate it now 1969 * without any locks in case it fails. 1970 */ 1971 if (refill_pi_state_cache()) 1972 return -ENOMEM; 1973 /* 1974 * requeue_pi must wake as many tasks as it can, up to nr_wake 1975 * + nr_requeue, since it acquires the rt_mutex prior to 1976 * returning to userspace, so as to not leave the rt_mutex with 1977 * waiters and no owner. However, second and third wake-ups 1978 * cannot be predicted as they involve race conditions with the 1979 * first wake and a fault while looking up the pi_state. Both 1980 * pthread_cond_signal() and pthread_cond_broadcast() should 1981 * use nr_wake=3D1. 1982 */ 1983 if (nr_wake !=3D 1) 1984 return -EINVAL; 1985 } 1986 = 1987 retry: 1988 ret =3D get_futex_key(uaddr1, flags & FLAGS_SHARED, &key1, FUTEX_RE= AD); 1989 if (unlikely(ret !=3D 0)) 1990 goto out; 1991 ret =3D get_futex_key(uaddr2, flags & FLAGS_SHARED, &key2, 1992 requeue_pi ? FUTEX_WRITE : FUTEX_READ); 1993 if (unlikely(ret !=3D 0)) 1994 goto out_put_key1; 1995 = 1996 /* 1997 * The check above which compares uaddrs is not sufficient for 1998 * shared futexes. We need to compare the keys: 1999 */ 2000 if (requeue_pi && match_futex(&key1, &key2)) { 2001 ret =3D -EINVAL; 2002 goto out_put_keys; 2003 } 2004 = 2005 hb1 =3D hash_futex(&key1); 2006 hb2 =3D hash_futex(&key2); 2007 = 2008 retry_private: 2009 hb_waiters_inc(hb2); 2010 double_lock_hb(hb1, hb2); 2011 = 2012 ret =3D get_futex_value_locked(&targetval, uaddr2); 2013 if (!ret && likely(cmpval !=3D NULL)) 2014 ret =3D get_futex_value_locked(&curval, uaddr1); 2015 = 2016 if (unlikely(ret)) { 2017 double_unlock_hb(hb1, hb2); 2018 hb_waiters_dec(hb2); 2019 = 2020 ret =3D get_user(targetval, uaddr2); 2021 if (!ret && likely(cmpval !=3D NULL)) 2022 ret =3D get_user(curval, uaddr1); 2023 = 2024 if (ret) 2025 goto out_put_keys; 2026 = 2027 if (!(flags & FLAGS_SHARED)) 2028 goto retry_private; 2029 = 2030 put_futex_key(&key2); 2031 put_futex_key(&key1); 2032 goto retry; 2033 } 2034 = 2035 if (likely(cmpval !=3D NULL) && (curval !=3D *cmpval)) { 2036 ret =3D -EAGAIN; 2037 goto out_unlock; 2038 } 2039 = 2040 if (requeue_pi && (task_count - nr_wake < nr_requeue)) { 2041 /* 2042 * Attempt to acquire uaddr2 and wake the top waiter. If we 2043 * intend to requeue waiters, force setting the FUTEX_WAITERS 2044 * bit. We force this here where we are able to easily handle 2045 * faults rather in the requeue loop below. 2046 */ 2047 ret =3D futex_proxy_trylock_atomic(uaddr2, hb1, hb2, &key1, 2048 &key2, &pi_state, nr_requeue); 2049 = 2050 /* 2051 * At this point the top_waiter has either taken uaddr2 or is 2052 * waiting on it. If the former, then the pi_state will not 2053 * exist yet, look it up one more time to ensure we have a 2054 * reference to it. If the lock was taken, ret contains the 2055 * vpid of the top waiter task. 2056 * If the lock was not taken, we have pi_state and an initial 2057 * refcount on it. In case of an error we have nothing. 2058 */ 2059 if (ret > 0) { 2060 WARN_ON(pi_state); 2061 drop_count++; 2062 task_count++; 2063 /* 2064 * If we acquired the lock, then the user space value 2065 * of uaddr2 should be vpid. It cannot be changed by 2066 * the top waiter as it is blocked on hb2 lock if it 2067 * tries to do so. If something fiddled with it behind 2068 * our back the pi state lookup might unearth it. So 2069 * we rather use the known value than rereading and 2070 * handing potential crap to lookup_pi_state. 2071 * 2072 * If that call succeeds then we have pi_state and an 2073 * initial refcount on it. 2074 */ 2075 ret =3D lookup_pi_state(uaddr2, ret, hb2, &key2, &pi_state); 2076 } 2077 = 2078 switch (ret) { 2079 case 0: 2080 /* We hold a reference on the pi state. */ 2081 break; 2082 = 2083 /* If the above failed, then pi_state is NULL */ 2084 case -EFAULT: 2085 double_unlock_hb(hb1, hb2); 2086 hb_waiters_dec(hb2); 2087 put_futex_key(&key2); 2088 put_futex_key(&key1); 2089 ret =3D fault_in_user_writeable(uaddr2); 2090 if (!ret) 2091 goto retry; 2092 goto out; 2093 case -EAGAIN: 2094 /* 2095 * Two reasons for this: 2096 * - Owner is exiting and we just wait for the 2097 * exit to complete. 2098 * - The user space value changed. 2099 */ 2100 double_unlock_hb(hb1, hb2); 2101 hb_waiters_dec(hb2); 2102 put_futex_key(&key2); 2103 put_futex_key(&key1); 2104 cond_resched(); 2105 goto retry; 2106 default: 2107 goto out_unlock; 2108 } 2109 } 2110 = 2111 plist_for_each_entry_safe(this, next, &hb1->chain, list) { 2112 if (task_count - nr_wake >=3D nr_requeue) 2113 break; 2114 = 2115 if (!match_futex(&this->key, &key1)) 2116 continue; 2117 = 2118 /* 2119 * FUTEX_WAIT_REQEUE_PI and FUTEX_CMP_REQUEUE_PI should always 2120 * be paired with each other and no other futex ops. 2121 * 2122 * We should never be requeueing a futex_q with a pi_state, 2123 * which is awaiting a futex_unlock_pi(). 2124 */ 2125 if ((requeue_pi && !this->rt_waiter) || 2126 (!requeue_pi && this->rt_waiter) || 2127 this->pi_state) { 2128 ret =3D -EINVAL; 2129 break; 2130 } 2131 = 2132 /* 2133 * Wake nr_wake waiters. For requeue_pi, if we acquired the 2134 * lock, we already woke the top_waiter. If not, it will be 2135 * woken by futex_unlock_pi(). 2136 */ 2137 if (++task_count <=3D nr_wake && !requeue_pi) { 2138 mark_wake_futex(&wake_q, this); 2139 continue; 2140 } 2141 = 2142 /* Ensure we requeue to the expected futex for requeue_pi. */ 2143 if (requeue_pi && !match_futex(this->requeue_pi_key, &key2)) { 2144 ret =3D -EINVAL; 2145 break; 2146 } 2147 = 2148 /* 2149 * Requeue nr_requeue waiters and possibly one more in the case 2150 * of requeue_pi if we couldn't acquire the lock atomically. 2151 */ 2152 if (requeue_pi) { 2153 /* 2154 * Prepare the waiter to take the rt_mutex. Take a 2155 * refcount on the pi_state and store the pointer in 2156 * the futex_q object of the waiter. 2157 */ 2158 get_pi_state(pi_state); 2159 this->pi_state =3D pi_state; 2160 ret =3D rt_mutex_start_proxy_lock(&pi_state->pi_mutex, 2161 this->rt_waiter, 2162 this->task); 2163 if (ret =3D=3D 1) { 2164 /* 2165 * We got the lock. We do neither drop the 2166 * refcount on pi_state nor clear 2167 * this->pi_state because the waiter needs the 2168 * pi_state for cleaning up the user space 2169 * value. It will drop the refcount after 2170 * doing so. 2171 */ 2172 requeue_pi_wake_futex(this, &key2, hb2); 2173 drop_count++; 2174 continue; 2175 } else if (ret) { 2176 /* 2177 * rt_mutex_start_proxy_lock() detected a 2178 * potential deadlock when we tried to queue 2179 * that waiter. Drop the pi_state reference 2180 * which we took above and remove the pointer 2181 * to the state from the waiters futex_q 2182 * object. 2183 */ 2184 this->pi_state =3D NULL; 2185 put_pi_state(pi_state); 2186 /* 2187 * We stop queueing more waiters and let user 2188 * space deal with the mess. 2189 */ 2190 break; 2191 } 2192 } 2193 requeue_futex(this, hb1, hb2, &key2); 2194 /* 2195 * Save target uaddr/val, in case futex_wait() wakes 2196 * up spuriously and retries to requeue. 2197 */ > 2198 this->uaddr =3D uaddr2; 2199 this->val =3D targetval; 2200 drop_count++; 2201 } 2202 = 2203 /* 2204 * We took an extra initial reference to the pi_state either 2205 * in futex_proxy_trylock_atomic() or in lookup_pi_state(). We 2206 * need to drop it here again. 2207 */ 2208 put_pi_state(pi_state); 2209 = 2210 out_unlock: 2211 double_unlock_hb(hb1, hb2); 2212 wake_up_q(&wake_q); 2213 hb_waiters_dec(hb2); 2214 = 2215 /* 2216 * drop_futex_key_refs() must be called outside the spinlocks. Duri= ng 2217 * the requeue we moved futex_q's from the hash bucket at key1 to t= he 2218 * one at key2 and updated their key pointer. We no longer need to 2219 * hold the references to key1. 2220 */ 2221 while (--drop_count >=3D 0) 2222 drop_futex_key_refs(&key1); 2223 = 2224 out_put_keys: 2225 put_futex_key(&key2); 2226 out_put_key1: 2227 put_futex_key(&key1); 2228 out: 2229 return ret ? ret : task_count; 2230 } 2231 = --- 0-DAY kernel test infrastructure Open Source Technology Cen= ter https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org Intel Corpor= ation --===============1940925121305421359==--