[U-Boot] A minor question on a Driver Model function

[Igor Grinberg <grinberg@compulab.co.il>]

On 09/18/14 18:46, Bill Pringlemeir wrote:
> 
>>>> On 12 September 2014 05:25, Masahiro Yamada
>>>> <yamada.m@jp.panasonic.com> wrote:
>>>
>>>>>>>> I have a qustion about lists_driver_lookup_name() function.
> 
>>>>>> On 09/14/14 21:28, Simon Glass wrote:
>>>
>>>>>> I would suggest still using strncmp as it is safer,
>>>>>> but count also the '\0', so something like:
>>>
>>> On 17 Sep 2014, grinberg at compulab.co.il wrote:
>>>
>>>>> Why safer?
>>>
>>>>> Could you give me more detailed explanation?
>>>
>>>> On 09/17/14 11:18, Masahiro Yamada wrote:
>>>
>>>> Well, I'm not an expert in s/w security, but I'll try to explain...
>>>
>>> [snip]
>>>
>>>> But, again, I'm not an expert in this area, so its only a
>>>> suggestion.
>>>
> 
>> On 09/17/14 18:25, Bill Pringlemeir wrote:
> 
>>> I thought it was fairly apparent that the current code supports
>>> passing a string that is *NOT* null terminated.  This can be
>>> convenient if you extract a sub-string from a command line and do not
>>> need to make a copy that is NULL terminate or perform 'strtok()' type
>>> magic.
> 
> On 18 Sep 2014, grinberg at compulab.co.il wrote:
> 
>> Here is the whole function:
>>
>> ------------------------------cut--------------------------
>> struct driver *lists_driver_lookup_name(const char *name)
>> {
>> struct driver *drv =
>> ll_entry_start(struct driver, driver);
>> const int n_ents = ll_entry_count(struct driver, driver);
>> struct driver *entry;
>> int len;
>>
>> if (!drv || !n_ents)
>> return NULL;
>>
>> len = strlen(name);
>>
>> for (entry = drv; entry != drv + n_ents; entry++) {
>> if (strncmp(name, entry->name, len))
>> continue;
>>
>> /* Full match */
>> if (len == strlen(entry->name))
>> return entry;
>>>
>>
>> /* Not found */
>> return NULL;
>>>
>> ------------------------------cut--------------------------
>>
>> and... no, the code does not support passing a string that is
>> not null terminated.
> 
> Then using the strncmp() seems useless for security reasons?  The 'len'
> is not passed in by the caller and 'strlen()' will have the same
> problems that 'strcmp()' would for read buffer overflows?  I would guess
> the code was cribbed from where 'len' was passed?  In that case, it
> would support strings that are not null terminated.

Yes, that is correct.

Since we are dealing with device/driver names here.
I think the best would be to define a sane name length limit
(say 20 or more characters) and use it as the maximal length
if no '\0' found before the limit.


-- 
Regards,
Igor.