Pull futex fix from Ingo Molnar: "A single fix for a robust futexes race between sys_exit() and sys_futex_lock_pi()" * 'locking-urgent-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: futex: Cure exit race
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -1148,11 +1148,65 @@ out_error: · 1147 unmodified lines | ||
| 1148 | 1148 | return ret; |
| 1149 | 1149 | } |
| 1150 | 1150 | |
| 1151 | +static int handle_exit_race(u32 __user *uaddr, u32 uval, | |
| 1152 | + struct task_struct *tsk) | |
| 1153 | +{ | |
| 1154 | + u32 uval2; | |
| 1155 | + | |
| 1156 | + /* | |
| 1157 | + * If PF_EXITPIDONE is not yet set, then try again. | |
| 1158 | + */ | |
| 1159 | + if (tsk && !(tsk->flags & PF_EXITPIDONE)) | |
| 1160 | + return -EAGAIN; | |
| 1161 | + | |
| 1162 | + /* | |
| 1163 | + * Reread the user space value to handle the following situation: | |
| 1164 | + * | |
| 1165 | + * CPU0 CPU1 | |
| 1166 | + * | |
| 1167 | + * sys_exit() sys_futex() | |
| 1168 | + * do_exit() futex_lock_pi() | |
| 1169 | + * futex_lock_pi_atomic() | |
| 1170 | + * exit_signals(tsk) No waiters: | |
| 1171 | + * tsk->flags |= PF_EXITING; *uaddr == 0x00000PID | |
| 1172 | + * mm_release(tsk) Set waiter bit | |
| 1173 | + * exit_robust_list(tsk) { *uaddr = 0x80000PID; | |
| 1174 | + * Set owner died attach_to_pi_owner() { | |
| 1175 | + * *uaddr = 0xC0000000; tsk = get_task(PID); | |
| 1176 | + * } if (!tsk->flags & PF_EXITING) { | |
| 1177 | + * ... attach(); | |
| 1178 | + * tsk->flags |= PF_EXITPIDONE; } else { | |
| 1179 | + * if (!(tsk->flags & PF_EXITPIDONE)) | |
| 1180 | + * return -EAGAIN; | |
| 1181 | + * return -ESRCH; <--- FAIL | |
| 1182 | + * } | |
| 1183 | + * | |
| 1184 | + * Returning ESRCH unconditionally is wrong here because the | |
| 1185 | + * user space value has been changed by the exiting task. | |
| 1186 | + * | |
| 1187 | + * The same logic applies to the case where the exiting task is | |
| 1188 | + * already gone. | |
| 1189 | + */ | |
| 1190 | + if (get_futex_value_locked(&uval2, uaddr)) | |
| 1191 | + return -EFAULT; | |
| 1192 | + | |
| 1193 | + /* If the user space value has changed, try again. */ | |
| 1194 | + if (uval2 != uval) | |
| 1195 | + return -EAGAIN; | |
| 1196 | + | |
| 1197 | + /* | |
| 1198 | + * The exiting task did not have a robust list, the robust list was | |
| 1199 | + * corrupted or the user space value in *uaddr is simply bogus. | |
| 1200 | + * Give up and tell user space. | |
| 1201 | + */ | |
| 1202 | + return -ESRCH; | |
| 1203 | +} | |
| 1204 | + | |
| 1151 | 1205 | /* |
| 1152 | 1206 | * Lookup the task for the TID provided from user space and attach to |
| 1153 | 1207 | * it after doing proper sanity checks. |
| 1154 | 1208 | */ |
| 1155 | −static int attach_to_pi_owner(u32 uval, union futex_key *key, | |
| 1209 | +static int attach_to_pi_owner(u32 __user *uaddr, u32 uval, union futex_key *key, | |
| 1156 | 1210 | struct futex_pi_state **ps) |
| 1157 | 1211 | { |
| 1158 | 1212 | pid_t pid = uval & FUTEX_TID_MASK; |
| @@ -1162,12 +1216,15 @@ static int attach_to_pi_owner(u32 uval, union futex_key *key, · 3 unmodified lines | ||
| 1162 | 1216 | /* |
| 1163 | 1217 | * We are the first waiter - try to look up the real owner and attach |
| 1164 | 1218 | * the new pi_state to it, but bail out when TID = 0 [1] |
| 1219 | + * | |
| 1220 | + * The !pid check is paranoid. None of the call sites should end up | |
| 1221 | + * with pid == 0, but better safe than sorry. Let the caller retry | |
| 1165 | 1222 | */ |
| 1166 | 1223 | if (!pid) |
| 1167 | − return -ESRCH; | |
| 1224 | + return -EAGAIN; | |
| 1168 | 1225 | p = find_get_task_by_vpid(pid); |
| 1169 | 1226 | if (!p) |
| 1170 | − return -ESRCH; | |
| 1227 | + return handle_exit_race(uaddr, uval, NULL); | |
| 1171 | 1228 | |
| 1172 | 1229 | if (unlikely(p->flags & PF_KTHREAD)) { |
| 1173 | 1230 | put_task_struct(p); |
| @@ -1187,7 +1244,7 @@ static int attach_to_pi_owner(u32 uval, union futex_key *key, · 13 unmodified lines | ||
| 1187 | 1244 | * set, we know that the task has finished the |
| 1188 | 1245 | * cleanup: |
| 1189 | 1246 | */ |
| 1190 | − int ret = (p->flags & PF_EXITPIDONE) ? -ESRCH : -EAGAIN; | |
| 1247 | + int ret = handle_exit_race(uaddr, uval, p); | |
| 1191 | 1248 | |
| 1192 | 1249 | raw_spin_unlock_irq(&p->pi_lock); |
| 1193 | 1250 | put_task_struct(p); |
| @@ -1244,7 +1301,7 @@ static int lookup_pi_state(u32 __user *uaddr, u32 uval, · 50 unmodified lines | ||
| 1244 | 1301 | * We are the first waiter - try to look up the owner based on |
| 1245 | 1302 | * @uval and attach to it. |
| 1246 | 1303 | */ |
| 1247 | − return attach_to_pi_owner(uval, key, ps); | |
| 1304 | + return attach_to_pi_owner(uaddr, uval, key, ps); | |
| 1248 | 1305 | } |
| 1249 | 1306 | |
| 1250 | 1307 | static int lock_pi_update_atomic(u32 __user *uaddr, u32 uval, u32 newval) |
| @@ -1352,7 +1409,7 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb, · 101 unmodified lines | ||
| 1352 | 1409 | * attach to the owner. If that fails, no harm done, we only |
| 1353 | 1410 | * set the FUTEX_WAITERS bit in the user space variable. |
| 1354 | 1411 | */ |
| 1355 | − return attach_to_pi_owner(uval, key, ps); | |
| 1412 | + return attach_to_pi_owner(uaddr, newval, key, ps); | |
| 1356 | 1413 | } |
| 1357 | 1414 | |
| 1358 | 1415 | /** |