[lkml]   [2005]   [Dec]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    Patch in this message
    SubjectRe: [PATCH] Fix user data corrupted by old value return of sysctl
    On Fri, Dec 30, 2005 at 09:25:35AM -0800, Linus Torvalds wrote:
    > On Fri, 30 Dec 2005, Yi Yang wrote:
    > >
    > > If the user reads a sysctl entry which is of string type
    > > by sysctl syscall, this call probably corrupts the user data
    > > right after the old value buffer, the issue lies in sysctl_string
    > > seting 0 to oldval[len], len is the available buffer size
    > > specified by the user, obviously, this will write to the first
    > > byte of the user memory place immediate after the old value buffer,
    > > the correct way is that sysctl_string doesn't set 0, the user
    > > should do it by self in the program.
    > Hmm.. I think this patch is incomplete.
    > We _should_ zero-pad the data, at least if the result fits in the buffer.
    > So I think the correct fix is to just _copy_ the last zero if it fits in
    > the buffer, rather than do the unconditional "add NUL at the end" thing.
    > The simplest way to do that is to just make "l" be "strlen(str)+1", so
    > that we count the ending NUL in the length (and then, if the buffer isn't
    > big enough, we will truncate it).
    > In other words, I would instead suggest a patch like the appended.
    > But even that is questionable: one alternative is to always zero-pad (like
    > we used to), but make sure that the buffer size is sufficient for it (ie
    > instead of adding one to the length of the string, we'd subtract one from
    > the buffer length and make sure that the '\0' fits..
    > Comments?

    Always do zero-pad please. I'd feel more comfortable with C strings (NULL
    terminated). Don't you?

    Poor code, also a small cleanup attached.

    Signed-off-by: Coywolf Qi Hunt <>

    diff --git a/kernel/sysctl.c b/kernel/sysctl.c
    index e5102ea..9960a26 100644
    --- a/kernel/sysctl.c
    +++ b/kernel/sysctl.c
    @@ -2203,7 +2203,7 @@ int sysctl_string(ctl_table *table, int
    if (len) {
    l = strlen(table->data)+1;
    if (len > l) len = l;
    - if (len >= table->maxlen)
    + if (len > table->maxlen)
    len = table->maxlen;
    if(copy_to_user(oldval, table->data, len))
    return -EFAULT;
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at
     \ /
      Last update: 2005-12-31 10:27    [W:0.021 / U:11.688 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site