[PATCH] env: mmc: Correct partition comparison in mmc_offset_try_partition

Wolfgang Denk wd at denx.de
Thu Nov 12 21:01:50 CET 2020


Dear Jaehoon Chung,

In message <21adc771-9660-da52-65c8-c2029de9a29e at samsung.com> you wrote:
> On 11/10/20 11:28 PM, Hoyeonjiki Kim wrote:
> > The function mmc_offset_try_partition searches MMC partition to save the
> > environment data by name.  However, it only compares the first word-size
> > bytes (size of 'const char *'), which may make the function to find
> > unintended partition.
> > 
> > Correct the function not to partially compare the partition name with
> > config "u-boot,,mmc-env-partition".
> > 
> > Signed-off-by: Hoyeonjiki Kim <jigi.kim at gmail.com>
> > ---
> >  env/mmc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/env/mmc.c b/env/mmc.c
> > index 4e67180b23..505f7aa2b8 100644
> > --- a/env/mmc.c
> > +++ b/env/mmc.c
> > @@ -42,7 +42,7 @@ static inline int mmc_offset_try_partition(const char *str, int copy, s64 *val)
> >  		if (ret < 0)
> >  			return ret;
> >  
> > -		if (!strncmp((const char *)info.name, str, sizeof(str)))
> > +		if (!strcmp((const char *)info.name, str))
>
> Using "strlen(str)" is better than changing to strcmp.
>
> strncmp(..., ..., strlen(str))

Is either of this a good idea? I mean, if you pass in random data,
this will run forever and eventually create undefined behaviour. We
know the maximum size, so why not limit it to that, as strncmp() did?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"To IBM, 'open' means there is a modicum  of  interoperability  among
some of their equipment."                            - Harv Masterson


More information about the U-Boot mailing list