lkml.org 
[lkml]   [2009]   [Aug]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 4/4] oom: fix oom_adjust_write() input sanity check.
    On Tue,  4 Aug 2009 19:28:03 +0900 (JST) KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote:

    > Subject: [PATCH] oom: fix oom_adjust_write() input sanity check.
    >
    > Andrew Morton pointed out oom_adjust_write() has very strange EIO
    > and new line handling. this patch fixes it.
    >
    >
    > Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
    > Cc: Paul Menage <menage@google.com>
    > Cc: David Rientjes <rientjes@google.com>
    > Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
    > Cc: Rik van Riel <riel@redhat.com>,
    > Cc: Andrew Morton <akpm@linux-foundation.org>,
    > ---
    > fs/proc/base.c | 12 +++++++-----
    > 1 file changed, 7 insertions(+), 5 deletions(-)
    >
    > Index: b/fs/proc/base.c
    > ===================================================================
    > --- a/fs/proc/base.c
    > +++ b/fs/proc/base.c
    > @@ -1033,12 +1033,15 @@ static ssize_t oom_adjust_write(struct f
    > count = sizeof(buffer) - 1;
    > if (copy_from_user(buffer, buf, count))
    > return -EFAULT;
    > +
    > + strstrip(buffer);

    +1 for using strstrip()

    -1 for using it wrongly. If it strips leading whitespace it will
    return a new address for the caller to use.

    We could mark it __must_check() to prevent reoccurences of this error.

    How does this look?

    --- a/fs/proc/base.c~oom-fix-oom_adjust_write-input-sanity-check-fix
    +++ a/fs/proc/base.c
    @@ -1033,8 +1033,7 @@ static ssize_t oom_adjust_write(struct f
    if (copy_from_user(buffer, buf, count))
    return -EFAULT;

    - strstrip(buffer);
    - oom_adjust = simple_strtol(buffer, &end, 0);
    + oom_adjust = simple_strtol(strstrip(buffer), &end, 0);
    if (*end)
    return -EINVAL;
    if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) &&

    > oom_adjust = simple_strtol(buffer, &end, 0);

    That should've used strict_strtoul() but it's too late to fix it now.

    > + if (*end)
    > + return -EINVAL;
    > if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) &&
    > oom_adjust != OOM_DISABLE)
    > return -EINVAL;
    > - if (*end == '\n')
    > - end++;
    > +
    > task = get_proc_task(file->f_path.dentry->d_inode);
    > if (!task)
    > return -ESRCH;
    > @@ -1057,9 +1060,8 @@ static ssize_t oom_adjust_write(struct f
    > task->signal->oom_adj = oom_adjust;
    > unlock_task_sighand(task, &flags);
    > put_task_struct(task);
    > - if (end - buffer == 0)
    > - return -EIO;
    > - return end - buffer;
    > +
    > + return count;
    > }
    >
    > static const struct file_operations proc_oom_adjust_operations = {
    >


    \
     
     \ /
      Last update: 2009-08-06 01:37    [W:0.030 / U:0.184 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site