diff options
author | Oleg Nesterov <oleg@tv-sign.ru> | 2008-02-01 09:29:05 -0500 |
---|---|---|
committer | Thomas Gleixner <tglx@linutronix.de> | 2008-02-10 04:48:03 -0500 |
commit | 080344b98805553f9b01de0f59a41b1533036d8d (patch) | |
tree | df56b37cca4b0ce233967682526158b58fa151b9 | |
parent | e13a2e61dd5152f5499d2003470acf9c838eab84 (diff) |
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(-)
-rw-r--r-- | include/linux/hrtimer.h | 2 | ||||
-rw-r--r-- | kernel/hrtimer.c | 51 | ||||
-rw-r--r-- | kernel/posix-timers.c | 17 |
3 files changed, 31 insertions, 39 deletions
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h index 600fc3bcf63e..1ad56a7b2f74 100644 --- a/include/linux/hrtimer.h +++ b/include/linux/hrtimer.h | |||
@@ -316,7 +316,7 @@ static inline u64 hrtimer_forward_now(struct hrtimer *timer, | |||
316 | 316 | ||
317 | /* Precise sleep: */ | 317 | /* Precise sleep: */ |
318 | extern long hrtimer_nanosleep(struct timespec *rqtp, | 318 | extern long hrtimer_nanosleep(struct timespec *rqtp, |
319 | struct timespec *rmtp, | 319 | struct timespec __user *rmtp, |
320 | const enum hrtimer_mode mode, | 320 | const enum hrtimer_mode mode, |
321 | const clockid_t clockid); | 321 | const clockid_t clockid); |
322 | extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); | 322 | extern long hrtimer_nanosleep_restart(struct restart_block *restart_block); |
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index 668f3967eb39..355085f0896e 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c | |||
@@ -1319,11 +1319,26 @@ static int __sched do_nanosleep(struct hrtimer_sleeper *t, enum hrtimer_mode mod | |||
1319 | return t->task == NULL; | 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 | long __sched hrtimer_nanosleep_restart(struct restart_block *restart) | 1338 | long __sched hrtimer_nanosleep_restart(struct restart_block *restart) |
1323 | { | 1339 | { |
1324 | struct hrtimer_sleeper t; | 1340 | struct hrtimer_sleeper t; |
1325 | struct timespec *rmtp; | 1341 | struct timespec __user *rmtp; |
1326 | ktime_t time; | ||
1327 | 1342 | ||
1328 | restart->fn = do_no_restart_syscall; | 1343 | restart->fn = do_no_restart_syscall; |
1329 | 1344 | ||
@@ -1333,12 +1348,11 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) | |||
1333 | if (do_nanosleep(&t, HRTIMER_MODE_ABS)) | 1348 | if (do_nanosleep(&t, HRTIMER_MODE_ABS)) |
1334 | return 0; | 1349 | return 0; |
1335 | 1350 | ||
1336 | rmtp = (struct timespec *)restart->arg1; | 1351 | rmtp = (struct timespec __user *)restart->arg1; |
1337 | if (rmtp) { | 1352 | if (rmtp) { |
1338 | time = ktime_sub(t.timer.expires, t.timer.base->get_time()); | 1353 | int ret = update_rmtp(&t.timer, rmtp); |
1339 | if (time.tv64 <= 0) | 1354 | if (ret <= 0) |
1340 | return 0; | 1355 | return ret; |
1341 | *rmtp = ktime_to_timespec(time); | ||
1342 | } | 1356 | } |
1343 | 1357 | ||
1344 | restart->fn = hrtimer_nanosleep_restart; | 1358 | restart->fn = hrtimer_nanosleep_restart; |
@@ -1347,12 +1361,11 @@ long __sched hrtimer_nanosleep_restart(struct restart_block *restart) | |||
1347 | return -ERESTART_RESTARTBLOCK; | 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 | const enum hrtimer_mode mode, const clockid_t clockid) | 1365 | const enum hrtimer_mode mode, const clockid_t clockid) |
1352 | { | 1366 | { |
1353 | struct restart_block *restart; | 1367 | struct restart_block *restart; |
1354 | struct hrtimer_sleeper t; | 1368 | struct hrtimer_sleeper t; |
1355 | ktime_t rem; | ||
1356 | 1369 | ||
1357 | hrtimer_init(&t.timer, clockid, mode); | 1370 | hrtimer_init(&t.timer, clockid, mode); |
1358 | t.timer.expires = timespec_to_ktime(*rqtp); | 1371 | t.timer.expires = timespec_to_ktime(*rqtp); |
@@ -1364,10 +1377,9 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, | |||
1364 | return -ERESTARTNOHAND; | 1377 | return -ERESTARTNOHAND; |
1365 | 1378 | ||
1366 | if (rmtp) { | 1379 | if (rmtp) { |
1367 | rem = ktime_sub(t.timer.expires, t.timer.base->get_time()); | 1380 | int ret = update_rmtp(&t.timer, rmtp); |
1368 | if (rem.tv64 <= 0) | 1381 | if (ret <= 0) |
1369 | return 0; | 1382 | return ret; |
1370 | *rmtp = ktime_to_timespec(rem); | ||
1371 | } | 1383 | } |
1372 | 1384 | ||
1373 | restart = ¤t_thread_info()->restart_block; | 1385 | restart = ¤t_thread_info()->restart_block; |
@@ -1383,8 +1395,7 @@ long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp, | |||
1383 | asmlinkage long | 1395 | asmlinkage long |
1384 | sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) | 1396 | sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) |
1385 | { | 1397 | { |
1386 | struct timespec tu, rmt; | 1398 | struct timespec tu; |
1387 | int ret; | ||
1388 | 1399 | ||
1389 | if (copy_from_user(&tu, rqtp, sizeof(tu))) | 1400 | if (copy_from_user(&tu, rqtp, sizeof(tu))) |
1390 | return -EFAULT; | 1401 | return -EFAULT; |
@@ -1392,15 +1403,7 @@ sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp) | |||
1392 | if (!timespec_valid(&tu)) | 1403 | if (!timespec_valid(&tu)) |
1393 | return -EINVAL; | 1404 | return -EINVAL; |
1394 | 1405 | ||
1395 | ret = hrtimer_nanosleep(&tu, rmtp ? &rmt : NULL, HRTIMER_MODE_REL, | 1406 | return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC); |
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; | ||
1404 | } | 1407 | } |
1405 | 1408 | ||
1406 | /* | 1409 | /* |
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index ce268966007d..022c9c3cee6f 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c | |||
@@ -982,20 +982,9 @@ sys_clock_getres(const clockid_t which_clock, struct timespec __user *tp) | |||
982 | static int common_nsleep(const clockid_t which_clock, int flags, | 982 | static int common_nsleep(const clockid_t which_clock, int flags, |
983 | struct timespec *tsave, struct timespec __user *rmtp) | 983 | struct timespec *tsave, struct timespec __user *rmtp) |
984 | { | 984 | { |
985 | struct timespec rmt; | 985 | return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ? |
986 | int ret; | 986 | HRTIMER_MODE_ABS : HRTIMER_MODE_REL, |
987 | 987 | which_clock); | |
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; | ||
999 | } | 988 | } |
1000 | 989 | ||
1001 | asmlinkage long | 990 | asmlinkage long |