grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH V2 3/4] ofnet: free memory on module unload


From: Andrei Borzenkov
Subject: Re: [PATCH V2 3/4] ofnet: free memory on module unload
Date: Sat, 10 Dec 2016 21:18:25 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1

02.12.2016 18:10, Stanislav Kholmanskikh пишет:
> On module unload each of its network cards are unregistered,
> but corresponding memory areas are not freed.
> 
> This commit is to fix this situation.
> 
> Freeing the transmit buffer goes via a special function, since
> it is allocated via ofnet_alloc_netbuf().
> 
> Signed-off-by: Stanislav Kholmanskikh <address@hidden>
> ---
>  grub-core/net/drivers/ieee1275/ofnet.c |   61 
> +++++++++++++++++++++++++++++++-
>  1 files changed, 60 insertions(+), 1 deletions(-)
> 
> diff --git a/grub-core/net/drivers/ieee1275/ofnet.c 
> b/grub-core/net/drivers/ieee1275/ofnet.c
> index 25559c8..1f8ac9a 100644
> --- a/grub-core/net/drivers/ieee1275/ofnet.c
> +++ b/grub-core/net/drivers/ieee1275/ofnet.c
> @@ -331,6 +331,40 @@ grub_ieee1275_alloc_mem (grub_size_t len)
>      return (void *)args.result;
>  }
>  
> +/* Free memory allocated by alloc-mem */
> +static grub_err_t
> +grub_ieee1275_free_mem (void *addr, grub_size_t len)
> +{
> +  struct free_args
> +  {
> +    struct grub_ieee1275_common_hdr common;
> +    grub_ieee1275_cell_t method;
> +    grub_ieee1275_cell_t len;
> +    grub_ieee1275_cell_t addr;
> +    grub_ieee1275_cell_t catch;
> +  }
> +  args;
> +
> +  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CANNOT_INTERPRET))
> +    {
> +      grub_error (GRUB_ERR_UNKNOWN_COMMAND, N_("interpret is not 
> supported"));
> +      return grub_errno;
> +    }
> +
> +  INIT_IEEE1275_COMMON (&args.common, "interpret", 3, 1);
> +  args.addr = (grub_ieee1275_cell_t)addr;
> +  args.len = len;
> +  args.method = (grub_ieee1275_cell_t) "free-mem";
> +
> +  if (IEEE1275_CALL_ENTRY_FN(&args) == -1 || args.catch)
> +    {
> +      grub_error (GRUB_ERR_INVALID_COMMAND, N_("free-mem failed"));
> +      return grub_errno;
> +    }
> +
> +  return GRUB_ERR_NONE;
> +}
> +
>  static void *
>  ofnet_alloc_netbuf (grub_size_t len)
>  {
> @@ -340,6 +374,15 @@ ofnet_alloc_netbuf (grub_size_t len)
>      return grub_zalloc (len);
>  }
>  
> +static void
> +ofnet_free_netbuf (void *addr, grub_size_t len)
> +{
> +  if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_VIRT_TO_REAL_BROKEN))
> +    grub_ieee1275_free_mem (addr, len);
> +  else
> +    grub_free (addr);
> +}
> +
>  static int
>  search_net_devices (struct grub_ieee1275_devalias *alias)
>  {
> @@ -494,9 +537,25 @@ GRUB_MOD_INIT(ofnet)
>  GRUB_MOD_FINI(ofnet)
>  {
>    struct grub_net_card *card, *next;
> +  struct grub_ofnetcard_data *ofdata;
>  
>    FOR_NET_CARDS_SAFE (card, next) 
>      if (card->driver && grub_strcmp (card->driver->name, "ofnet") == 0)
> -      grub_net_card_unregister (card);
> +      {
> +     grub_net_card_unregister (card);
> +     /*
> +      * The fact that we are here means the card was successfully
> +      * initialized in the past, so all the below pointers are valid,
> +      * and we may free associated memory without checks.
> +      */
> +     ofdata = (struct grub_ofnetcard_data *) card->data;
> +     grub_free (ofdata->path);
> +     grub_free (ofdata);
> +
> +     ofnet_free_netbuf (card->txbuf, card->txbufsize);
> +
> +     grub_free ((void *) card->name);
> +     grub_free (card);
> +      }

No, it's not safe to do. I plunged into it in efinet and reverted. We
may have dangling references to card left, so you cannot free it (at
least, please not at this point before release and not without prior
audit). See

commit cc699535e57e0d0f099090e64a63037c7834f104
Author: Andrei Borzenkov <address@hidden>
Date:   Mon May 4 09:13:53 2015 +0300

    Revert "efinet: memory leak on module removal"


>    grub_ieee1275_net_config = 0;
>  }
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]