On Friday, December 27, 2019 at 2:33:19 PM UTC+3, Jan Kiszka wrote:
>
> On 24.12.19 11:40, Hosgor, Tolga (IOT DS EU TR MTS) wrote: 
> > From: Hosgor, Tolga (IOT DS EU TR MTS) <[email protected] 
> <javascript:>> 
> > 
> > In cases where get_mountpoint() returns non-null, do_unmount is not set 
> > to true and umount_partition() is not called. This results in 
> > cfgpart->mountpoint to not be freed. 
> > 
> > Signed-off-by: Hosgor, Tolga (IOT DS EU TR MTS) <[email protected] 
> <javascript:>> 
> > --- 
> >   env/env_config_file.c                | 3 +++ 
> >   tools/tests/test_probe_config_file.c | 1 - 
> >   2 files changed, 3 insertions(+), 1 deletion(-) 
> > 
> > diff --git a/env/env_config_file.c b/env/env_config_file.c 
> > index 873fe10..9b96f20 100644 
> > --- a/env/env_config_file.c 
> > +++ b/env/env_config_file.c 
> > @@ -88,6 +88,9 @@ bool probe_config_file(CONFIG_PART *cfgpart) 
> >                   } 
> >                   if (do_unmount) { 
> >                           unmount_partition(cfgpart); 
> > +                } else { 
> > +                        free(cfgpart->mountpoint); 
> > +                        cfgpart->mountpoint = NULL; 
>
> This looks wrong. 
>
 

> There are places in the code that uses 
> cfgpart->pointpoint after probe_config_file in case cfgpart->not_mounted 
> is false. Check env/env_api_fat.c.

I see that now.


> I rather suspect, we are missing a counterpart to bgenv_init() that 
> cleans config_parts up. 
>
Yes indeed. Later I found out that `devpath` is also leaking due to 
repeated calls to bgenv_init. Handling both in the same place would be 
better but I could not really decide how to implement that (where to call 
it). Instead, I have set the CONFIG_PARTs to {0} and tried to free them 
prior to memset in bgenv_init as a dirty workaround.


> Jan 
>
> >                   } 
> >                   return result; 
> >           } 
> > diff --git a/tools/tests/test_probe_config_file.c 
> b/tools/tests/test_probe_config_file.c 
> > index f1da154..9b1a176 100644 
> > --- a/tools/tests/test_probe_config_file.c 
> > +++ b/tools/tests/test_probe_config_file.c 
> > @@ -120,7 +120,6 @@ bool __wrap_probe_config_file(CONFIG_PART *cp) 
> >           cp->not_mounted = false; 
> >           ret =  __real_probe_config_file(cp); 
> > 
> > -        free(cp->mountpoint); 
> >           return ret; 
> >   } 
> > 
> > 
>
>

-- 
You received this message because you are subscribed to the Google Groups "EFI 
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/efibootguard-dev/f1a483dd-7de4-4c9f-8076-ed84470d1331%40googlegroups.com.

Reply via email to