Re: [PATCH 2/2] tracing/filters: use strcmp() instead of strncmp()

From: Li Zefan
Date: Mon Jun 01 2009 - 01:44:40 EST


>>>> 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.

>
>
>> 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.
>>
>>
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/