lkml.org 
[lkml]   [2008]   [Feb]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[patch 25/38] hrtimer: fix *rmtp handling in hrtimer_nanosleep()
    2.6.24-stable review patch.  If anyone has any objections, please let us
    know.

    ------------------
    From: Oleg Nesterov <oleg@tv-sign.ru>

    commit 080344b98805553f9b01de0f59a41b1533036d8d

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

    ---
    include/linux/hrtimer.h | 2 -
    kernel/hrtimer.c | 51 +++++++++++++++++++++++++-----------------------
    kernel/posix-timers.c | 17 ++--------------
    3 files changed, 31 insertions(+), 39 deletions(-)

    --- a/include/linux/hrtimer.h
    +++ b/include/linux/hrtimer.h
    @@ -300,7 +300,7 @@ hrtimer_forward(struct hrtimer *timer, k

    /* Precise sleep: */
    extern long hrtimer_nanosleep(struct timespec *rqtp,
    - struct timespec *rmtp,
    + struct timespec __user *rmtp,
    const enum hrtimer_mode mode,
    const clockid_t clockid);
    extern long hrtimer_nanosleep_restart(struct restart_block *restart_block);
    --- a/kernel/hrtimer.c
    +++ b/kernel/hrtimer.c
    @@ -1291,11 +1291,26 @@ static int __sched do_nanosleep(struct h
    return t->task == NULL;
    }

    +static int update_rmtp(struct hrtimer *timer, struct timespec __user *rmtp)
    +{
    + struct timespec rmt;
    + ktime_t rem;
    +
    + rem = ktime_sub(timer->expires, timer->base->get_time());
    + if (rem.tv64 <= 0)
    + return 0;
    + rmt = ktime_to_timespec(rem);
    +
    + if (copy_to_user(rmtp, &rmt, sizeof(*rmtp)))
    + return -EFAULT;
    +
    + return 1;
    +}
    +
    long __sched hrtimer_nanosleep_restart(struct restart_block *restart)
    {
    struct hrtimer_sleeper t;
    - struct timespec *rmtp;
    - ktime_t time;
    + struct timespec __user *rmtp;

    restart->fn = do_no_restart_syscall;

    @@ -1305,12 +1320,11 @@ long __sched hrtimer_nanosleep_restart(s
    if (do_nanosleep(&t, HRTIMER_MODE_ABS))
    return 0;

    - rmtp = (struct timespec *)restart->arg1;
    + rmtp = (struct timespec __user *)restart->arg1;
    if (rmtp) {
    - time = ktime_sub(t.timer.expires, t.timer.base->get_time());
    - if (time.tv64 <= 0)
    - return 0;
    - *rmtp = ktime_to_timespec(time);
    + int ret = update_rmtp(&t.timer, rmtp);
    + if (ret <= 0)
    + return ret;
    }

    restart->fn = hrtimer_nanosleep_restart;
    @@ -1319,12 +1333,11 @@ long __sched hrtimer_nanosleep_restart(s
    return -ERESTART_RESTARTBLOCK;
    }

    -long hrtimer_nanosleep(struct timespec *rqtp, struct timespec *rmtp,
    +long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp,
    const enum hrtimer_mode mode, const clockid_t clockid)
    {
    struct restart_block *restart;
    struct hrtimer_sleeper t;
    - ktime_t rem;

    hrtimer_init(&t.timer, clockid, mode);
    t.timer.expires = timespec_to_ktime(*rqtp);
    @@ -1336,10 +1349,9 @@ long hrtimer_nanosleep(struct timespec *
    return -ERESTARTNOHAND;

    if (rmtp) {
    - rem = ktime_sub(t.timer.expires, t.timer.base->get_time());
    - if (rem.tv64 <= 0)
    - return 0;
    - *rmtp = ktime_to_timespec(rem);
    + int ret = update_rmtp(&t.timer, rmtp);
    + if (ret <= 0)
    + return ret;
    }

    restart = &current_thread_info()->restart_block;
    @@ -1355,8 +1367,7 @@ long hrtimer_nanosleep(struct timespec *
    asmlinkage long
    sys_nanosleep(struct timespec __user *rqtp, struct timespec __user *rmtp)
    {
    - struct timespec tu, rmt;
    - int ret;
    + struct timespec tu;

    if (copy_from_user(&tu, rqtp, sizeof(tu)))
    return -EFAULT;
    @@ -1364,15 +1375,7 @@ sys_nanosleep(struct timespec __user *rq
    if (!timespec_valid(&tu))
    return -EINVAL;

    - ret = hrtimer_nanosleep(&tu, rmtp ? &rmt : NULL, HRTIMER_MODE_REL,
    - CLOCK_MONOTONIC);
    -
    - if (ret && rmtp) {
    - if (copy_to_user(rmtp, &rmt, sizeof(*rmtp)))
    - return -EFAULT;
    - }
    -
    - return ret;
    + return hrtimer_nanosleep(&tu, rmtp, HRTIMER_MODE_REL, CLOCK_MONOTONIC);
    }

    /*
    --- a/kernel/posix-timers.c
    +++ b/kernel/posix-timers.c
    @@ -981,20 +981,9 @@ sys_clock_getres(const clockid_t which_c
    static int common_nsleep(const clockid_t which_clock, int flags,
    struct timespec *tsave, struct timespec __user *rmtp)
    {
    - struct timespec rmt;
    - int ret;
    -
    - ret = hrtimer_nanosleep(tsave, rmtp ? &rmt : NULL,
    - flags & TIMER_ABSTIME ?
    - HRTIMER_MODE_ABS : HRTIMER_MODE_REL,
    - which_clock);
    -
    - if (ret && rmtp) {
    - if (copy_to_user(rmtp, &rmt, sizeof(*rmtp)))
    - return -EFAULT;
    - }
    -
    - return ret;
    + return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ?
    + HRTIMER_MODE_ABS : HRTIMER_MODE_REL,
    + which_clock);
    }

    asmlinkage long
    --


    \
     
     \ /
      Last update: 2008-02-23 01:49    [W:2.290 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site