lkml.org 
[lkml]   [2009]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp()
    On Mon, Jun 01, 2009 at 01:45:47PM +0800, Li Zefan wrote:
    > >>>> I don't think there's any security issue. It's irrelevant how big the user-input
    > >>>> strings are. The point is those strings are guaranteed to be NULL-terminated.
    > >>>> Am I missing something?
    > >>>>
    > >>>> And I don't think it's necessary to make 2 patches that each patch converts
    > >>>> one strncmp to strcmp. But maybe it's better to improve this changelog?
    > >>> Hmm, you must be right, indeed they seem to be guaranted beeing NULL-terminated
    > >>> strings.
    > >>>
    > >> Sorry, I was wrong. :(
    > >>
    > >> Though the user-input strings are guaranted to be NULL-terminated, strings
    > >> generated by TRACE_EVENT might not.
    > >>
    > >> We define static strings this way:
    > >> TP_struct(
    > >> __array(char, foo, LEN)
    > >> )
    > >> But foo is not necessarily a string, though I doubt someone will use it
    > >> as non-string char array.
    > >
    > >
    > > Yeah, but the user defined comparison operand is NULL terminated.
    > > So the strcmp will stop at this boundary.
    > >
    >
    > The user input string is NULL terminated and is limited to MAX_FILTER_STR_VAL,
    > and it's strcmp() not strcpy(), but it's still unsafe. No?
    >
    > cmp = strcmp(addr, pred->str_val);
    >
    > If addr is not NULL-terminated string but char array, and length of
    > str_val > length of addr, then we'll be exceeding the boundary of the
    > array.



    No, once both strings appear to be different, strcmp returns.
    As an example, the generic strcmp in lib/string.c is as follows:

    int strcmp(const char *cs, const char *ct)
    {
    signed char __res;

    while (1) {
    if ((__res = *cs - *ct++) != 0 || !*cs++)
    break;
    }
    return __res;
    }

    Once cs[n] != ct[n], or !cs[n] || !ct[n], strcmp() stops,
    and the x86 implementation does exactly the same.

    So I guess it's safe.


    > >
    > >
    > >> Dynamic string is fine, because assign_str() makes it NULL-terminated.
    > >>
    > >> So we can use strcmp() for dynamic strings, but we'd better use strncmp() for
    > >> static string.
    > >>
    > >>
    > >
    > >
    > >



    \
     
     \ /
      Last update: 2009-06-01 15:13    [W:0.029 / U:0.900 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site