From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Fu, Siyuan" <siyuan.fu@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ye, Ting" <ting.ye@intel.com>, "Wang, Fan" <fan.wang@intel.com>
Subject: Re: [Patch] MdeModulePkg/IpIoLib: Check the input parameters before use them.
Date: Thu, 21 Dec 2017 08:47:58 +0000 [thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B727416355AEB@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20171213080712.1404-1-siyuan.fu@intel.com>
Reviewed-by: Jiaxin Wu <jiaxin.wu@intel.com>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Fu,
> Siyuan
> Sent: Wednesday, December 13, 2017 4:07 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wang, Fan <fan.wang@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [edk2] [Patch] MdeModulePkg/IpIoLib: Check the input parameters
> before use them.
>
> This patch updates the DxeIpIoLib to check the input parameters before
> using.
>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Cc: Wang Fan <fan.wang@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan <siyuan.fu@intel.com>
> ---
> MdeModulePkg/Include/Library/IpIoLib.h | 20 ++++----
> MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 73
> ++++++++++++++++++++++------
> 2 files changed, 68 insertions(+), 25 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Library/IpIoLib.h
> b/MdeModulePkg/Include/Library/IpIoLib.h
> index aab0c68059..a57bc582d6 100644
> --- a/MdeModulePkg/Include/Library/IpIoLib.h
> +++ b/MdeModulePkg/Include/Library/IpIoLib.h
> @@ -2,7 +2,7 @@
> This library is only intended to be used by UEFI network stack modules.
> It provides the combined IpIo layer on the EFI IP4 Protocol and EFI IP6
> protocol.
>
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials are licensed and made
> available under
> the terms and conditions of the BSD License that accompanies this
> distribution.
> The full text of the license may be found at
> @@ -359,8 +359,9 @@ IpIoDestroy (
>
> @param[in, out] IpIo The pointer to the IP_IO instance that needs to
> stop.
>
> - @retval EFI_SUCCESS The IP_IO instance stopped successfully.
> - @retval Others Anrror condition occurred.
> + @retval EFI_SUCCESS The IP_IO instance stopped successfully.
> + @retval EFI_INVALID_PARAMETER Invalid input parameter.
> + @retval Others Error condition occurred.
>
> **/
> EFI_STATUS
> @@ -381,11 +382,12 @@ IpIoStop (
> @param[in] OpenData The configuration data and callbacks for
> the IP_IO instance.
>
> - @retval EFI_SUCCESS The IP_IO instance opened with OpenData
> - successfully.
> - @retval EFI_ACCESS_DENIED The IP_IO instance is configured; avoid
> - reopening it.
> - @retval Others An error condition occurred.
> + @retval EFI_SUCCESS The IP_IO instance opened with OpenData
> + successfully.
> + @retval EFI_ACCESS_DENIED The IP_IO instance is configured,
> avoid to
> + reopen it.
> + @retval EFI_INVALID_PARAMETER Invalid input parameter.
> + @retval Others Error condition occurred.
>
> **/
> EFI_STATUS
> @@ -518,7 +520,7 @@ IpIoRemoveIp (
> @param[in] Src The local IP address.
>
> @return The pointer to the IP protocol can be used for sending purpose
> and its local
> - address is the same with Src.
> + address is the same with Src. NULL if failed.
>
> **/
> IP_IO_IP_INFO *
> diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> index abc07fb0ff..33e2863419 100644
> --- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> +++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
> @@ -2,7 +2,7 @@
> IpIo Library.
>
> (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> -Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD
> License
> which accompanies this distribution. The full text of the license may be
> found at
> @@ -280,15 +280,22 @@ IpIoIcmpv4Handler (
> UINT8 Type;
> UINT8 Code;
> UINT32 TrimBytes;
> -
> +
> + ASSERT (IpIo != NULL);
> + ASSERT (Pkt != NULL);
> + ASSERT (Session != NULL);
> ASSERT (IpIo->IpVersion == IP_VERSION_4);
> -
> - IcmpHdr = NET_PROTO_HDR (Pkt, IP4_ICMP_ERROR_HEAD);
> - IpHdr = (EFI_IP4_HEADER *) (&IcmpHdr->IpHead);
> -
> +
> //
> // Check the ICMP packet length.
> //
> + if (Pkt->TotalSize < sizeof (IP4_ICMP_ERROR_HEAD)) {
> + return EFI_ABORTED;
> + }
> +
> + IcmpHdr = NET_PROTO_HDR (Pkt, IP4_ICMP_ERROR_HEAD);
> + IpHdr = (EFI_IP4_HEADER *) (&IcmpHdr->IpHead);
> +
> if (Pkt->TotalSize < ICMP_ERRLEN (IpHdr)) {
>
> return EFI_ABORTED;
> @@ -412,6 +419,9 @@ IpIoIcmpv6Handler (
> UINT32 TrimBytes;
> BOOLEAN Flag;
>
> + ASSERT (IpIo != NULL);
> + ASSERT (Pkt != NULL);
> + ASSERT (Session != NULL);
> ASSERT (IpIo->IpVersion == IP_VERSION_6);
>
> //
> @@ -1028,6 +1038,7 @@ IpIoListenHandlerDpc (
> }
>
> if (IpIo->IpVersion == IP_VERSION_4) {
> + ASSERT (RxData->Ip4RxData.Header != NULL);
> if (IP4_IS_LOCAL_BROADCAST (EFI_IP4 (RxData->Ip4RxData.Header-
> >SourceAddress))) {
> //
> // The source address is a broadcast address, discard it.
> @@ -1052,6 +1063,11 @@ IpIoListenHandlerDpc (
> }
>
> //
> + // The fragment should always be valid for non-zero length packet.
> + //
> + ASSERT (RxData->Ip4RxData.FragmentCount != 0);
> +
> + //
> // Create a netbuffer representing IPv4 packet
> //
> Pkt = NetbufFromExt (
> @@ -1075,7 +1091,7 @@ IpIoListenHandlerDpc (
> Session.IpHdrLen = RxData->Ip4RxData.HeaderLength;
> Session.IpVersion = IP_VERSION_4;
> } else {
> -
> + ASSERT (RxData->Ip6RxData.Header != NULL);
> if (!NetIp6IsValidUnicast(&RxData->Ip6RxData.Header->SourceAddress)) {
> goto CleanUp;
> }
> @@ -1088,6 +1104,11 @@ IpIoListenHandlerDpc (
> }
>
> //
> + // The fragment should always be valid for non-zero length packet.
> + //
> + ASSERT (RxData->Ip6RxData.FragmentCount != 0);
> +
> + //
> // Create a netbuffer representing IPv6 packet
> //
> Pkt = NetbufFromExt (
> @@ -1272,11 +1293,12 @@ ReleaseIpIo:
> @param[in] OpenData The configuration data and callbacks for
> the IP_IO instance.
>
> - @retval EFI_SUCCESS The IP_IO instance opened with OpenData
> - successfully.
> - @retval EFI_ACCESS_DENIED The IP_IO instance is configured, avoid
> to
> - reopen it.
> - @retval Others Error condition occurred.
> + @retval EFI_SUCCESS The IP_IO instance opened with OpenData
> + successfully.
> + @retval EFI_ACCESS_DENIED The IP_IO instance is configured,
> avoid to
> + reopen it.
> + @retval EFI_INVALID_PARAMETER Invalid input parameter.
> + @retval Others Error condition occurred.
>
> **/
> EFI_STATUS
> @@ -1289,6 +1311,10 @@ IpIoOpen (
> EFI_STATUS Status;
> UINT8 IpVersion;
>
> + if (IpIo == NULL || OpenData == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> if (IpIo->IsConfigured) {
> return EFI_ACCESS_DENIED;
> }
> @@ -1400,8 +1426,9 @@ ErrorExit:
>
> @param[in, out] IpIo Pointer to the IP_IO instance that needs to stop.
>
> - @retval EFI_SUCCESS The IP_IO instance stopped successfully.
> - @retval Others Error condition occurred.
> + @retval EFI_SUCCESS The IP_IO instance stopped successfully.
> + @retval EFI_INVALID_PARAMETER Invalid input parameter.
> + @retval Others Error condition occurred.
>
> **/
> EFI_STATUS
> @@ -1414,6 +1441,10 @@ IpIoStop (
> IP_IO_IP_INFO *IpInfo;
> UINT8 IpVersion;
>
> + if (IpIo == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> if (!IpIo->IsConfigured) {
> return EFI_SUCCESS;
> }
> @@ -1916,6 +1947,10 @@ IpIoRemoveIp (
> {
>
> UINT8 IpVersion;
> +
> + if (IpIo == NULL || IpInfo == NULL) {
> + return;
> + }
>
> ASSERT (IpInfo->RefCnt > 0);
>
> @@ -1980,7 +2015,7 @@ IpIoRemoveIp (
> @param[in] Src The local IP address.
>
> @return Pointer to the IP protocol can be used for sending purpose and its
> local
> - address is the same with Src.
> + address is the same with Src. NULL if failed.
>
> **/
> IP_IO_IP_INFO *
> @@ -1996,7 +2031,13 @@ IpIoFindSender (
> LIST_ENTRY *IpInfoEntry;
> IP_IO_IP_INFO *IpInfo;
>
> - ASSERT ((IpVersion == IP_VERSION_4) || (IpVersion == IP_VERSION_6));
> + if (IpIo == NULL || Src == NULL) {
> + return NULL;
> + }
> +
> + if ((IpVersion != IP_VERSION_4) && (IpVersion != IP_VERSION_6)) {
> + return NULL;
> + }
>
> NET_LIST_FOR_EACH (IpIoEntry, &mActiveIpIoList) {
> IpIoPtr = NET_LIST_USER_STRUCT (IpIoEntry, IP_IO, Entry);
> --
> 2.13.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
prev parent reply other threads:[~2017-12-21 8:43 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-13 8:07 [Patch] MdeModulePkg/IpIoLib: Check the input parameters before use them Fu Siyuan
2017-12-21 8:47 ` Wu, Jiaxin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=895558F6EA4E3B41AC93A00D163B727416355AEB@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox