From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1cGNMF-0006kb-QB for mharc-grub-devel@gnu.org; Mon, 12 Dec 2016 04:57:19 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37828) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cGNM4-0006bv-2C for grub-devel@gnu.org; Mon, 12 Dec 2016 04:57:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cGNM0-0002AO-Vg for grub-devel@gnu.org; Mon, 12 Dec 2016 04:57:08 -0500 Received: from userp1040.oracle.com ([156.151.31.81]:22826) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cGNM0-000295-O0 for grub-devel@gnu.org; Mon, 12 Dec 2016 04:57:04 -0500 Received: from userv0022.oracle.com (userv0022.oracle.com [156.151.31.74]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id uBC9v2sv017181 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 12 Dec 2016 09:57:02 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by userv0022.oracle.com (8.14.4/8.14.4) with ESMTP id uBC9v1mO031971 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Mon, 12 Dec 2016 09:57:02 GMT Received: from abhmp0001.oracle.com (abhmp0001.oracle.com [141.146.116.7]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id uBC9v1C8029858 for ; Mon, 12 Dec 2016 09:57:01 GMT Received: from [10.162.81.165] (/10.162.81.165) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Mon, 12 Dec 2016 01:57:00 -0800 Subject: Re: [PATCH V2 3/4] ofnet: free memory on module unload To: The development of GNU GRUB References: <1480691405-15118-1-git-send-email-stanislav.kholmanskikh@oracle.com> <1480691405-15118-4-git-send-email-stanislav.kholmanskikh@oracle.com> <150829c3-3493-a2b1-8891-1d49875f7451@gmail.com> Cc: vasily.isaenko@oracle.com From: Stanislav Kholmanskikh Message-ID: <584E7469.4010003@oracle.com> 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 MIME-Version: 1.0 In-Reply-To: <150829c3-3493-a2b1-8891-1d49875f7451@gmail.com> Content-Type: text/plain; charset=utf-8 X-Source-IP: userv0022.oracle.com [156.151.31.74] Content-Transfer-Encoding: quoted-printable X-MIME-Autoconverted: from 8bit to quoted-printable by userp1040.oracle.com id uBC9v2sv017181 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] [fuzzy] X-Received-From: 156.151.31.81 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Dec 2016 09:57:09 -0000 On 12/10/2016 09:18 PM, Andrei Borzenkov wrote: > 02.12.2016 18:10, Stanislav Kholmanskikh =D0=BF=D0=B8=D1=88=D0=B5=D1=82= : >> 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 >> --- >> 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/dr= ivers/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; >> } >> =20 >> +/* 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 supp= orted")); >> + return grub_errno; >> + } >> + >> + INIT_IEEE1275_COMMON (&args.common, "interpret", 3, 1); >> + args.addr =3D (grub_ieee1275_cell_t)addr; >> + args.len =3D len; >> + args.method =3D (grub_ieee1275_cell_t) "free-mem"; >> + >> + if (IEEE1275_CALL_ENTRY_FN(&args) =3D=3D -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); >> } >> =20 >> +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; >> =20 >> FOR_NET_CARDS_SAFE (card, next)=20 >> if (card->driver && grub_strcmp (card->driver->name, "ofnet") =3D= =3D 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 =3D (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); >> + } >=20 > 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 >=20 > commit cc699535e57e0d0f099090e64a63037c7834f104 > Author: Andrei Borzenkov > Date: Mon May 4 09:13:53 2015 +0300 >=20 > Revert "efinet: memory leak on module removal" >=20 Thanks. This is sad. I'll exclude these changes from V3 then. >=20 >> grub_ieee1275_net_config =3D 0; >> } >> >=20 >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >=20