Commit ad133ba3dc283300e5b62b5b7211d2f39fbf6ee7
Committed by
Ingo Molnar
1 parent
29d7b90c15
Exists in
master
and in
39 other branches
sched, signals: fix the racy usage of ->signal in account_group_xxx/run_posix_cpu_timers
Impact: fix potential NULL dereference Contrary to ad474caca3e2a0550b7ce0706527ad5ab389a4d4 changelog, other acct_group_xxx() helpers can be called after exit_notify() by timer tick. Thanks to Roland for pointing out this. Somehow I missed this simple fact when I read the original patch, and I am afraid I confused Frank during the discussion. Sorry. Fortunately, these helpers work with current, we can check ->exit_state to ensure that ->signal can't go away under us. Also, add the comment and compiler barrier to account_group_exec_runtime(), to make sure we load ->signal only once. Signed-off-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Ingo Molnar <mingo@elte.hu>
Showing 2 changed files with 16 additions and 6 deletions Side-by-side Diff
kernel/posix-cpu-timers.c
... | ... | @@ -1308,9 +1308,10 @@ |
1308 | 1308 | */ |
1309 | 1309 | static inline int fastpath_timer_check(struct task_struct *tsk) |
1310 | 1310 | { |
1311 | - struct signal_struct *sig = tsk->signal; | |
1311 | + struct signal_struct *sig; | |
1312 | 1312 | |
1313 | - if (unlikely(!sig)) | |
1313 | + /* tsk == current, ensure it is safe to use ->signal/sighand */ | |
1314 | + if (unlikely(tsk->exit_state)) | |
1314 | 1315 | return 0; |
1315 | 1316 | |
1316 | 1317 | if (!task_cputime_zero(&tsk->cputime_expires)) { |
... | ... | @@ -1323,6 +1324,8 @@ |
1323 | 1324 | if (task_cputime_expired(&task_sample, &tsk->cputime_expires)) |
1324 | 1325 | return 1; |
1325 | 1326 | } |
1327 | + | |
1328 | + sig = tsk->signal; | |
1326 | 1329 | if (!task_cputime_zero(&sig->cputime_expires)) { |
1327 | 1330 | struct task_cputime group_sample; |
1328 | 1331 |
kernel/sched_stats.h
... | ... | @@ -298,9 +298,11 @@ |
298 | 298 | { |
299 | 299 | struct signal_struct *sig; |
300 | 300 | |
301 | - sig = tsk->signal; | |
302 | - if (unlikely(!sig)) | |
301 | + /* tsk == current, ensure it is safe to use ->signal */ | |
302 | + if (unlikely(tsk->exit_state)) | |
303 | 303 | return; |
304 | + | |
305 | + sig = tsk->signal; | |
304 | 306 | if (sig->cputime.totals) { |
305 | 307 | struct task_cputime *times; |
306 | 308 | |
307 | 309 | |
... | ... | @@ -325,9 +327,11 @@ |
325 | 327 | { |
326 | 328 | struct signal_struct *sig; |
327 | 329 | |
328 | - sig = tsk->signal; | |
329 | - if (unlikely(!sig)) | |
330 | + /* tsk == current, ensure it is safe to use ->signal */ | |
331 | + if (unlikely(tsk->exit_state)) | |
330 | 332 | return; |
333 | + | |
334 | + sig = tsk->signal; | |
331 | 335 | if (sig->cputime.totals) { |
332 | 336 | struct task_cputime *times; |
333 | 337 | |
334 | 338 | |
... | ... | @@ -353,8 +357,11 @@ |
353 | 357 | struct signal_struct *sig; |
354 | 358 | |
355 | 359 | sig = tsk->signal; |
360 | + /* see __exit_signal()->task_rq_unlock_wait() */ | |
361 | + barrier(); | |
356 | 362 | if (unlikely(!sig)) |
357 | 363 | return; |
364 | + | |
358 | 365 | if (sig->cputime.totals) { |
359 | 366 | struct task_cputime *times; |
360 | 367 |