[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V2 3/4] ofnet: free memory on module unload
From: |
Stanislav Kholmanskikh |
Subject: |
Re: [PATCH V2 3/4] ofnet: free memory on module unload |
Date: |
Mon, 12 Dec 2016 12:56:57 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 12/10/2016 09:18 PM, Andrei Borzenkov wrote:
> 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"
>
Thanks. This is sad.
I'll exclude these changes from V3 then.
>
>> grub_ieee1275_net_config = 0;
>> }
>>
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
[PATCH V2 4/4] ofnet: implement the receive buffer, Stanislav Kholmanskikh, 2016/12/02