From mboxrd@z Thu Jan 1 00:00:00 1970 From: Igor Grinberg Date: Fri, 19 Sep 2014 09:34:53 +0300 Subject: [U-Boot] A minor question on a Driver Model function In-Reply-To: <87egv9x779.fsf@nbsps.com> References: <54169D84.9030400@compulab.co.il> <20140917171856.3BCB.AA925319@jp.panasonic.com> <54198F86.3080802@compulab.co.il> <87oauez2t4.fsf@nbsps.com> <541AD248.6000100@compulab.co.il> <87egv9x779.fsf@nbsps.com> Message-ID: <541BCE8D.4010009@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09/18/14 18:46, Bill Pringlemeir wrote: > >>>> On 12 September 2014 05:25, Masahiro Yamada >>>> 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.