Hi, Lubo
Some comments as below:
1.
Dhcp6Dxe,DnsDxe\ComponentName.c
HttpBootDxe\HttpBootDhcp6.c
UefiPxeBcDxe\PxeBcDhcp6.c
Check for NULL pointer before free it.
2.
DnsDxe\DnsImpl.c
Ip6Dxe\ComponentName.c
Mtftp6Dxe\Mtftp6Support.c
There is memory leak if GetModeData return SUCCESS but IsConfigured is
False.
3.
IpSecDxe\IkeService.c
Mtftp6Dxe\Mtftp6Support.c
Ip6ModeData is not initialized but used in ON_EXIT.
4.
// The caller is responsible for freeing the reference buffer
The comments seems redundant. It's just one line FreePool.
Siyuan
-----Original Message-----
From: Zhang, Lubo
Sent: Thursday, January 21, 2016 10:07 AM
To: edk2-devel@lists.01.org
Cc: Fu, Siyuan <siyuan...@intel.com>; Ye, Ting <ting...@intel.com>; Wu, Jiaxin
<jiaxin...@intel.com>
Subject: [patch] NetworkPkg:Fix Network memory leak when calling GetModeData
interface
Multiple network protocols have a GetModeData() interface, which may allocate
memory resource in the return mode data structure. It's callers responsibility
to free these buffers.
Cc: Fu Siyuan <siyuan...@intel.com>
Cc: Ye Ting <ting...@intel.com>
Cc: Wu Jiaxin <jiaxin...@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo <lubo.zh...@intel.com>
---
NetworkPkg/Dhcp6Dxe/ComponentName.c | 7 ++++++-
NetworkPkg/DnsDxe/ComponentName.c | 11 +++++++++++
NetworkPkg/DnsDxe/DnsDhcp.c | 13 ++++++++++++-
NetworkPkg/DnsDxe/DnsImpl.c | 35 ++++++++++++++++++++++++++++++++++
NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 6 +++++-
NetworkPkg/Ip6Dxe/ComponentName.c | 27 ++++++++++++++++++++++++++
NetworkPkg/IpSecDxe/IkeService.c | 27 ++++++++++++++++++++++++++
NetworkPkg/Mtftp6Dxe/Mtftp6Support.c | 26 +++++++++++++++++++++++++
NetworkPkg/TcpDxe/TcpMisc.c | 30 ++++++++++++++++++++++++++++-
NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c | 7 ++++++-
NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 34 +++++++++++++++++++++++++++++++--
11 files changed, 216 insertions(+), 7 deletions(-)
diff --git a/NetworkPkg/Dhcp6Dxe/ComponentName.c
b/NetworkPkg/Dhcp6Dxe/ComponentName.c
index 927a7fe..e46106d 100644
--- a/NetworkPkg/Dhcp6Dxe/ComponentName.c
+++ b/NetworkPkg/Dhcp6Dxe/ComponentName.c
@@ -284,11 +284,16 @@ UpdateName (
if (Dhcp6ModeData.Ia->State > Dhcp6Rebinding) {
return EFI_DEVICE_ERROR;
}
HandleName = mDhcp6ControllerName[Dhcp6ModeData.Ia->State];
}
-
+ //
+ // The caller is responsible for freeing the reference buffer //
+ FreePool (Dhcp6ModeData.ClientId); FreePool (Dhcp6ModeData.Ia);
+
Status = AddUnicodeString2 (
"eng",
gDhcp6ComponentName.SupportedLanguages,
&gDhcp6ControllerNameTable,
HandleName,
diff --git a/NetworkPkg/DnsDxe/ComponentName.c
b/NetworkPkg/DnsDxe/ComponentName.c
index d95ed92..1efd425 100644
--- a/NetworkPkg/DnsDxe/ComponentName.c
+++ b/NetworkPkg/DnsDxe/ComponentName.c
@@ -209,10 +209,16 @@ UpdateDns4Name (
ModeData.DnsConfigData.StationIp.Addr[2],
ModeData.DnsConfigData.StationIp.Addr[3],
ModeData.DnsConfigData.LocalPort
);
+ //
+ // The caller is responsible for freeing the reference buffer //
+ FreePool (ModeData.DnsCacheList); FreePool (ModeData.DnsServerList);
+
if (gDnsControllerNameTable != NULL) {
FreeUnicodeStringTable (gDnsControllerNameTable);
gDnsControllerNameTable = NULL;
}
@@ -278,10 +284,15 @@ UpdateDns6Name (
sizeof (HandleName),
L"DNSv6 (StationIp=%s, LocalPort=%d)",
Address,
ModeData.DnsConfigData.LocalPort
);
+ //
+ // The caller is responsible for freeing the reference buffer //
+ FreePool (ModeData.DnsServerList); FreePool (ModeData.DnsCacheList);
if (gDnsControllerNameTable != NULL) {
FreeUnicodeStringTable (gDnsControllerNameTable);
gDnsControllerNameTable = NULL;
}
diff --git a/NetworkPkg/DnsDxe/DnsDhcp.c b/NetworkPkg/DnsDxe/DnsDhcp.c index
6b409ba..71fb698 100644
--- a/NetworkPkg/DnsDxe/DnsDhcp.c
+++ b/NetworkPkg/DnsDxe/DnsDhcp.c
@@ -131,11 +131,22 @@ DnsStartIp4(
while (!Timeout) {
Ip4->Poll (Ip4);
if (!EFI_ERROR (Ip4->GetModeData (Ip4, &Ip4Mode, NULL, NULL)) &&
- Ip4Mode.IsConfigured) {
+ Ip4Mode.IsConfigured) {
+ if (Ip4Mode.GroupTable != NULL) {
+ FreePool (Ip4Mode.GroupTable);
+ }
+
+ if (Ip4Mode.RouteTable != NULL) {
+ FreePool (Ip4Mode.RouteTable);
+ }
+
+ if (Ip4Mode.IcmpTypeList != NULL) {
+ FreePool (Ip4Mode.IcmpTypeList);
+ }
break;
}
}
if (Timeout) {
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index
71dacce..e5cbfc6 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -568,10 +568,21 @@ Dns4GetMapping (
while (!EFI_ERROR (gBS->CheckEvent (Service->TimerToGetMap))) {
Udp->Poll (Udp);
if (!EFI_ERROR (Udp->GetModeData (Udp, NULL, &Ip4Mode, NULL, NULL)) &&
Ip4Mode.IsConfigured) {
+ if (Ip4Mode.GroupTable != NULL) {
+ FreePool (Ip4Mode.GroupTable);
+ }
+
+ if (Ip4Mode.RouteTable != NULL) {
+ FreePool (Ip4Mode.RouteTable);
+ }
+
+ if (Ip4Mode.IcmpTypeList != NULL) {
+ FreePool (Ip4Mode.IcmpTypeList);
+ }
Udp->Configure (Udp, NULL);
return (BOOLEAN) (Udp->Configure (Udp, UdpCfgData) == EFI_SUCCESS);
}
}
@@ -619,10 +630,34 @@ Dns6GetMapping (
Udp->Poll (Udp);
if (!EFI_ERROR (Udp->GetModeData (Udp, NULL, &Ip6Mode, NULL, NULL)) &&
Ip6Mode.IsConfigured) {
+ if (Ip6Mode.AddressList != NULL) {
+ FreePool (Ip6Mode.AddressList);
+ }
+
+ if (Ip6Mode.GroupTable != NULL) {
+ FreePool (Ip6Mode.GroupTable);
+ }
+
+ if (Ip6Mode.RouteTable != NULL) {
+ FreePool (Ip6Mode.RouteTable);
+ }
+
+ if (Ip6Mode.NeighborCache != NULL) {
+ FreePool (Ip6Mode.NeighborCache);
+ }
+
+ if (Ip6Mode.PrefixTable != NULL) {
+ FreePool (Ip6Mode.PrefixTable);
+ }
+
+ if (Ip6Mode.IcmpTypeList != NULL) {
+ FreePool (Ip6Mode.IcmpTypeList);
+ }
+
Udp->Configure (Udp, NULL);
return (BOOLEAN) (Udp->Configure (Udp, UdpCfgData) == EFI_SUCCESS);
}
}
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
index e5cf894..3067f2d 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootDhcp6.c
@@ -972,12 +972,16 @@ ON_EXIT:
if (EFI_ERROR (Status)) {
Dhcp6->Stop (Dhcp6);
Dhcp6->Configure (Dhcp6, NULL);
} else {
ZeroMem (&Config, sizeof (EFI_DHCP6_CONFIG_DATA));
- ZeroMem (&Mode, sizeof (EFI_DHCP6_MODE_DATA));
Dhcp6->Configure (Dhcp6, &Config);
+ //
+ // The caller is responsible for freeing the reference buffer
+ //
+ FreePool (Mode.ClientId);
+ FreePool (Mode.Ia);
}
return Status;
}
diff --git a/NetworkPkg/Ip6Dxe/ComponentName.c
b/NetworkPkg/Ip6Dxe/ComponentName.c
index 75a1562..103d081 100644
--- a/NetworkPkg/Ip6Dxe/ComponentName.c
+++ b/NetworkPkg/Ip6Dxe/ComponentName.c
@@ -262,10 +262,37 @@ UpdateName (
// Format the child name into the string buffer.
//
Offset = 0;
Status = Ip6->GetModeData (Ip6, &Ip6ModeData, NULL, NULL);
if (!EFI_ERROR (Status) && Ip6ModeData.IsStarted) {
+ //
+ // The caller need to free the reference buffer
+ //
+ if (Ip6ModeData.AddressList != NULL) {
+ FreePool (Ip6ModeData.AddressList);
+ }
+
+ if (Ip6ModeData.GroupTable != NULL) {
+ FreePool (Ip6ModeData.GroupTable);
+ }
+
+ if (Ip6ModeData.RouteTable != NULL) {
+ FreePool (Ip6ModeData.RouteTable);
+ }
+
+ if (Ip6ModeData.NeighborCache != NULL) {
+ FreePool (Ip6ModeData.NeighborCache);
+ }
+
+ if (Ip6ModeData.PrefixTable != NULL) {
+ FreePool (Ip6ModeData.PrefixTable);
+ }
+
+ if (Ip6ModeData.IcmpTypeList != NULL) {
+ FreePool (Ip6ModeData.IcmpTypeList);
+ }
+
Status = NetLibIp6ToStr (&Ip6ModeData.ConfigData.StationAddress, Address,
sizeof(Address));
if (EFI_ERROR (Status)) {
return Status;
}
Offset += UnicodeSPrint (
diff --git a/NetworkPkg/IpSecDxe/IkeService.c b/NetworkPkg/IpSecDxe/IkeService.c
index d857196..2803a89 100644
--- a/NetworkPkg/IpSecDxe/IkeService.c
+++ b/NetworkPkg/IpSecDxe/IkeService.c
@@ -345,10 +345,37 @@ IkeOpenOutputUdp (
}
UdpService->IsConfigured = TRUE;
ON_EXIT:
+ //
+ // The caller need to free the reference buffer // if
+ (Ip6ModeData.AddressList != NULL) {
+ FreePool (Ip6ModeData.AddressList); }
+
+ if (Ip6ModeData.GroupTable != NULL) {
+ FreePool (Ip6ModeData.GroupTable);
+ }
+
+ if (Ip6ModeData.RouteTable != NULL) {
+ FreePool (Ip6ModeData.RouteTable);
+ }
+
+ if (Ip6ModeData.NeighborCache != NULL) {
+ FreePool (Ip6ModeData.NeighborCache); }
+
+ if (Ip6ModeData.PrefixTable != NULL) {
+ FreePool (Ip6ModeData.PrefixTable); }
+
+ if (Ip6ModeData.IcmpTypeList != NULL) {
+ FreePool (Ip6ModeData.IcmpTypeList); }
+
if (IfInfo != NULL) {
FreePool (IfInfo);
}
return Status;
diff --git a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
index c31fc9d..9487d30 100644
--- a/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
+++ b/NetworkPkg/Mtftp6Dxe/Mtftp6Support.c
@@ -330,10 +330,36 @@ Mtftp6GetMapping (
}
}
}
ON_EXIT:
+ //
+ // The caller need to free the reference buffer // if
+ (Ip6Mode.AddressList != NULL) {
+ FreePool (Ip6Mode.AddressList);
+ }
+
+ if (Ip6Mode.GroupTable != NULL) {
+ FreePool (Ip6Mode.GroupTable);
+ }
+
+ if (Ip6Mode.RouteTable != NULL) {
+ FreePool (Ip6Mode.RouteTable);
+ }
+
+ if (Ip6Mode.NeighborCache != NULL) {
+ FreePool (Ip6Mode.NeighborCache);
+ }
+
+ if (Ip6Mode.PrefixTable != NULL) {
+ FreePool (Ip6Mode.PrefixTable);
+ }
+
+ if (Ip6Mode.IcmpTypeList != NULL) {
+ FreePool (Ip6Mode.IcmpTypeList);
+ }
if (Event != NULL) {
gBS->CloseEvent (Event);
}
diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c index
4096252..a8fdc78 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -559,17 +559,45 @@ TcpGetRcvMss (
if (Sock->IpVersion == IP_VERSION_4) {
Ip4 = TcpProto->TcpService->IpIo->Ip.Ip4;
ASSERT (Ip4 != NULL);
Ip4->GetModeData (Ip4, &Ip4Mode, NULL, NULL);
+ if (Ip4Mode.GroupTable != NULL) {
+ FreePool (Ip4Mode.GroupTable);
+ }
+
+ if (Ip4Mode.RouteTable != NULL) {
+ FreePool (Ip4Mode.RouteTable);
+ }
+
+ if (Ip4Mode.IcmpTypeList != NULL) {
+ FreePool (Ip4Mode.IcmpTypeList);
+ }
return (UINT16) (Ip4Mode.MaxPacketSize - sizeof (TCP_HEAD));
} else {
Ip6 = TcpProto->TcpService->IpIo->Ip.Ip6;
ASSERT (Ip6 != NULL);
Ip6->GetModeData (Ip6, &Ip6Mode, NULL, NULL);
-
+ if (Ip6Mode.AddressList != NULL) {
+ FreePool (Ip6Mode.AddressList);
+ }
+ if (Ip6Mode.GroupTable != NULL) {
+ FreePool (Ip6Mode.GroupTable);
+ }
+ if (Ip6Mode.RouteTable != NULL) {
+ FreePool (Ip6Mode.RouteTable);
+ }
+ if (Ip6Mode.NeighborCache != NULL) {
+ FreePool (Ip6Mode.NeighborCache);
+ }
+ if (Ip6Mode.PrefixTable != NULL) {
+ FreePool (Ip6Mode.PrefixTable);
+ }
+ if (Ip6Mode.IcmpTypeList != NULL) {
+ FreePool (Ip6Mode.IcmpTypeList);
+ }
return (UINT16) (Ip6Mode.MaxPacketSize - sizeof (TCP_HEAD));
}
}
/**
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
index 6ad5f5f..fe15950 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDhcp6.c
@@ -2080,17 +2080,22 @@ PxeBcDhcp6Sarr (
// the IP policy is Automatic). So we just hold the station IP address here
and leave the IP policy as
// Automatic, until we get the server IP address. This could let IP6 driver
finish the router discovery
// to find a valid router address.
//
CopyMem (&Private->TmpStationIp.v6, &Mode.Ia->IaAddress[0].IpAddress, sizeof
(EFI_IPv6_ADDRESS));
+ //
+ // The caller is responsible for freeing the reference buffer //
+ FreePool (Mode.ClientId); FreePool (Mode.Ia);
//
// Check the selected offer whether BINL retry is needed.
//
Status = PxeBcHandleDhcp6Offer (Private);
if (EFI_ERROR (Status)) {
Dhcp6->Stop (Dhcp6);
return Status;
}
-
+
return EFI_SUCCESS;
}
diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
index a6f6668..e125768 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
@@ -729,10 +729,22 @@ PxeBcCreateIp4Children (
//
Status = Private->Ip4->GetModeData (Private->Ip4, &Ip4ModeData, NULL, NULL);
if (EFI_ERROR (Status)) {
goto ON_ERROR;
}
+
+ if (Ip4ModeData.GroupTable != NULL) {
+ FreePool (Ip4ModeData.GroupTable);
+ }
+
+ if (Ip4ModeData.RouteTable != NULL) {
+ FreePool (Ip4ModeData.RouteTable);
+ }
+
+ if (Ip4ModeData.IcmpTypeList != NULL) {
+ FreePool (Ip4ModeData.IcmpTypeList); }
Private->Ip4MaxPacketSize = Ip4ModeData.MaxPacketSize;
Private->Ip4Nic = AllocateZeroPool (sizeof (PXEBC_VIRTUAL_NIC));
if (Private->Ip4Nic == NULL) {
@@ -1056,13 +1068,31 @@ PxeBcCreateIp6Children (
//
Status = Private->Ip6->GetModeData (Private->Ip6, &Ip6ModeData, NULL, NULL);
if (EFI_ERROR (Status)) {
goto ON_ERROR;
}
-
+
Private->Ip6MaxPacketSize = Ip6ModeData.MaxPacketSize;
-
+
+ if (Ip6ModeData.AddressList != NULL) {
+ FreePool (Ip6ModeData.AddressList); } if (Ip6ModeData.GroupTable
+ != NULL) {
+ FreePool (Ip6ModeData.GroupTable);
+ }
+ if (Ip6ModeData.RouteTable != NULL) {
+ FreePool (Ip6ModeData.RouteTable);
+ }
+ if (Ip6ModeData.NeighborCache != NULL) {
+ FreePool (Ip6ModeData.NeighborCache); } if
+ (Ip6ModeData.PrefixTable != NULL) {
+ FreePool (Ip6ModeData.PrefixTable); } if
+ (Ip6ModeData.IcmpTypeList != NULL) {
+ FreePool (Ip6ModeData.IcmpTypeList); }
//
// Locate Ip6->Ip6Config and store it for set IPv6 address.
//
Status = gBS->HandleProtocol (
ControllerHandle,
--
1.9.5.msysgit.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
