Commit 080344b98805553f9b01de0f59a41b1533036d8d
Committed by
Thomas Gleixner
1 parent
e13a2e61dd
Exists in
master
and in
40 other branches
hrtimer: fix *rmtp handling in hrtimer_nanosleep()
Spotted by Pavel Emelyanov and Alexey Dobriyan. hrtimer_nanosleep() sets restart_block->arg1 = rmtp, but this rmtp points to the local variable which lives in the caller's stack frame. This means that if sys_restart_syscall() actually happens and it is interrupted as well, we don't update the user-space variable, but write into the already dead stack frame. Introduced by commit 04c227140fed77587432667a574b14736a06dd7f hrtimer: Rework hrtimer_nanosleep to make sys_compat_nanosleep easier Change the callers to pass "__user *rmtp" to hrtimer_nanosleep(), and change hrtimer_nanosleep() to use copy_to_user() to actually update *rmtp. Small problem remains. man 2 nanosleep states that *rtmp should be written if nanosleep() was interrupted (it says nothing whether it is OK to update *rmtp if nanosleep returns 0), but (with or without this patch) we can dirty *rem even if nanosleep() returns 0. NOTE: this patch doesn't change compat_sys_nanosleep(), because it has other bugs. Fixed by the next patch. Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru> Cc: Alexey Dobriyan <adobriyan@sw.ru> Cc: Michael Kerrisk <mtk.manpages@googlemail.com> Cc: Pavel Emelyanov <xemul@sw.ru> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl> Cc: Toyo Abe <toyoa@mvista.com> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> include/linux/hrtimer.h | 2 - kernel/hrtimer.c | 51 +++++++++++++++++++++++++----------------------- kernel/posix-timers.c | 14 +------------ 3 files changed, 30 insertions(+), 37 deletions(-)
Showing 3 changed files with 31 additions and 39 deletions Side-by-side Diff
include/linux/hrtimer.h
... | ... | @@ -316,7 +316,7 @@ |
316 | 316 | |
317 | 317 | /* Precise sleep: */ |
318 | 318 | extern long hrtimer_nanosleep(struct timespec *rqtp, |
319 | - struct timespec *rmtp, | |
319 | + struct timespec __user *rmtp, | |
320 | 320 | const enum hrtimer_mode mode, |
321 | 321 | const clockid_t clockid); |
322 | 322 | extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); |
kernel/hrtimer.c
... | ... | @@ -1319,11 +1319,26 @@ |
1319 | 1319 | return t->task == NULL; |
1320 | 1320 | } |
1321 | 1321 | |
1322 | +static int update_rmtp(struct hrtimer *timer, struct timespec __user *rmtp) | |
1323 | +{ | |
1324 | + struct timespec rmt; | |
1325 | + ktime_t rem; | |
1326 | + | |
1327 | + rem = ktime_sub(timer->expires, timer->base->get_time()); | |
1328 | + if (rem.tv64 <= 0) | |
1329 | + return 0; | |
1330 | + rmt = ktime_to_timespec(rem); | |
1331 | + | |
1332 | + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) | |
1333 | + return -EFAULT; | |
1334 | + | |
1335 | + return 1; | |
1336 | +} | |
1337 | + | |
1322 | 1338 | long __sched hrtimer_nanosleep_restart(struct restart_block *restart) |
1323 | 1339 | { |
1324 | 1340 | struct hrtimer_sleeper t; |
1325 | - struct timespec *rmtp; | |
1326 | - ktime_t time; | |
1341 | + struct timespec __user *rmtp; | |
1327 | 1342 | |
1328 | 1343 | restart->fn = do_no_restart_syscall; |
1329 | 1344 | |
1330 | 1345 | |
... | ... | @@ -1333,12 +1348,11 @@ |
1333 | 1348 | if (do_nanosleep(&t, HRTIMER_MODE_ABS)) |
1334 | 1349 | return 0; |
1335 | 1350 | |
1336 | - rmtp = (struct timespec *)restart->arg1; | |
1351 | + rmtp = (struct timespec __user *)restart->arg1; | |
1337 | 1352 | if (rmtp) { |
1338 | - time = ktime_sub(t.timer.expires, t.timer.base->get_time()); | |
1339 | - if (time.tv64 <= 0) | |
1340 | - return 0; | |
1341 | - *rmtp = ktime_to_timespec(time); | |
1353 | + int ret = update_rmtp(&t.timer, rmtp); | |
1354 | + if (ret <= 0) | |
1355 | + return ret; | |
1342 | 1356 | } |
1343 | 1357 | |
1344 | 1358 | restart->fn = hrtimer_nanosleep_restart; |
1345 | 1359 | |
... | ... | @@ -1347,12 +1361,11 @@ |
1347 | 1361 | return -ERESTART_RESTARTBLOCK; |
1348 | 1362 | } |
1349 | 1363 | |
1350 | -long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, | |
1364 | +long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp, | |
1351 | 1365 | const enum hrtimer_mode mode, const clockid_t clockid) |
1352 | 1366 | { |
1353 | 1367 | struct restart_block *restart; |
1354 | 1368 | struct hrtimer_sleeper t; |
1355 | - ktime_t rem; | |
1356 | 1369 | |
1357 | 1370 | hrtimer_init(&t.timer, clockid, mode); |
1358 | 1371 | t.timer.expires = timespec_to_ktime(*rqtp); |
... | ... | @@ -1364,10 +1377,9 @@ |
1364 | 1377 | return -ERESTARTNOHAND; |
1365 | 1378 | |
1366 | 1379 | if (rmtp) { |
1367 | - rem = ktime_sub(t.timer.expires, t.timer.base->get_time()); | |
1368 | - if (rem.tv64 <= 0) | |
1369 | - return 0; | |
1370 | - *rmtp = ktime_to_timespec(rem); | |
1380 | + int ret = update_rmtp(&t.timer, rmtp); | |
1381 | + if (ret <= 0) | |
1382 | + return ret; | |
1371 | 1383 | } |
1372 | 1384 | |
1373 | 1385 | restart = ¤t_thread_info()->restart_block; |
... | ... | @@ -1383,8 +1395,7 @@ |
1383 | 1395 | asmlinkage long |
1384 | 1396 | sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) |
1385 | 1397 | { |
1386 | - struct timespec tu, rmt; | |
1387 | - int ret; | |
1398 | + struct timespec tu; | |
1388 | 1399 | |
1389 | 1400 | if (copy_from_user(&tu, rqtp, sizeof(tu))) |
1390 | 1401 | return -EFAULT; |
... | ... | @@ -1392,15 +1403,7 @@ |
1392 | 1403 | if (!timespec_valid(&tu)) |
1393 | 1404 | return -EINVAL; |
1394 | 1405 | |
1395 | - ret = hrtimer_nanosleep(&tu, rmtp ? &rmt : NULL, HRTIMER_MODE_REL, | |
1396 | - CLOCK_MONOTONIC); | |
1397 | - | |
1398 | - if (ret && rmtp) { | |
1399 | - if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) | |
1400 | - return -EFAULT; | |
1401 | - } | |
1402 | - | |
1403 | - return ret; | |
1406 | + return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); | |
1404 | 1407 | } |
1405 | 1408 | |
1406 | 1409 | /* |
kernel/posix-timers.c
... | ... | @@ -982,20 +982,9 @@ |
982 | 982 | static int common_nsleep(const clockid_t which_clock, int flags, |
983 | 983 | struct timespec *tsave, struct timespec __user *rmtp) |
984 | 984 | { |
985 | - struct timespec rmt; | |
986 | - int ret; | |
987 | - | |
988 | - ret = hrtimer_nanosleep(tsave, rmtp ? &rmt : NULL, | |
989 | - flags & TIMER_ABSTIME ? | |
990 | - HRTIMER_MODE_ABS : HRTIMER_MODE_REL, | |
991 | - which_clock); | |
992 | - | |
993 | - if (ret && rmtp) { | |
994 | - if (copy_to_user(rmtp, &rmt, sizeof(*rmtp))) | |
995 | - return -EFAULT; | |
996 | - } | |
997 | - | |
998 | - return ret; | |
985 | + return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? | |
986 | + HRTIMER_MODE_ABS : HRTIMER_MODE_REL, | |
987 | + which_clock); | |
999 | 988 | } |
1000 | 989 | |
1001 | 990 | asmlinkage long |