[Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id before use it directly.

[Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id before use it directly.

[Jiaxin Wu]

Nii->Id is the address of the first byte of the identifying structure
for this network interface. This is only valid when the network interface
is started. When the network interface is not started, this field is set
to zero. So, we should check the value of Nii->Id before use it directly.

Cc: Wang Fan <fan.wang@intel.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
index 9f61aee..f0257a2 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
@@ -1,9 +1,9 @@
 /** @file
   Implementation of driver entry point and driver binding protocol.
 
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -333,10 +333,15 @@ SimpleNetworkDriverStart (
     return Status;
   }
 
   DEBUG ((EFI_D_INFO, "Start(): UNDI3.1 found\n"));
 
+  if (Nii->Id == 0) {
+    DEBUG ((EFI_D_NET, "\nUNDI is not started.\n"));
+    goto NiiError;
+  }
+
   Pxe = (PXE_UNDI *) (UINTN) (Nii->Id);
 
   if (Calc8BitCksum (Pxe, Pxe->hw.Len) != 0) {
     DEBUG ((EFI_D_NET, "\n!PXE checksum is not correct.\n"));
     goto NiiError;
-- 
1.9.5.msysgit.1

Re: [Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id before use it directly.

[Fu, Siyuan]

Hi, Jiaxin

In the line you are modifying the UDNI is always not started. You should move the Nii->Id to the line after PxeStart(), not to check it here.

BestRegards
Fu Siyuan

> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Thursday, December 21, 2017 3:13 PM
> To: edk2-devel@lists.01.org
> Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id before
> use it directly.
> 
> Nii->Id is the address of the first byte of the identifying structure
> for this network interface. This is only valid when the network interface
> is started. When the network interface is not started, this field is set
> to zero. So, we should check the value of Nii->Id before use it directly.
> 
> Cc: Wang Fan <fan.wang@intel.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/SnpDxe/Snp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> index 9f61aee..f0257a2 100644
> --- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> +++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> @@ -1,9 +1,9 @@
>  /** @file
>    Implementation of driver entry point and driver binding protocol.
> 
> -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2004 - 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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -333,10 +333,15 @@ SimpleNetworkDriverStart (
>      return Status;
>    }
> 
>    DEBUG ((EFI_D_INFO, "Start(): UNDI3.1 found\n"));
> 
> +  if (Nii->Id == 0) {
> +    DEBUG ((EFI_D_NET, "\nUNDI is not started.\n"));
> +    goto NiiError;
> +  }
> +
>    Pxe = (PXE_UNDI *) (UINTN) (Nii->Id);
> 
>    if (Calc8BitCksum (Pxe, Pxe->hw.Len) != 0) {
>      DEBUG ((EFI_D_NET, "\n!PXE checksum is not correct.\n"));
>      goto NiiError;
> --
> 1.9.5.msysgit.1

Re: [Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id before use it directly.

[Wu, Jiaxin]

Thanks to catch that.  The UNDI is started by SNP driver. Then, Nii->Id changes to the none-zero value. But currently, UNDI seems not follow that.  


> -----Original Message-----
> From: Fu, Siyuan
> Sent: Thursday, December 21, 2017 3:39 PM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: RE: [Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id
> before use it directly.
> 
> Hi, Jiaxin
> 
> In the line you are modifying the UDNI is always not started. You should
> move the Nii->Id to the line after PxeStart(), not to check it here.
> 
> BestRegards
> Fu Siyuan
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Thursday, December 21, 2017 3:13 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>; Fu,
> > Siyuan <siyuan.fu@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>
> > Subject: [Patch] MdeModulePkg/SnpDxe: Check the value of Nii->Id
> before
> > use it directly.
> >
> > Nii->Id is the address of the first byte of the identifying structure
> > for this network interface. This is only valid when the network interface
> > is started. When the network interface is not started, this field is set
> > to zero. So, we should check the value of Nii->Id before use it directly.
> >
> > Cc: Wang Fan <fan.wang@intel.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > ---
> >  MdeModulePkg/Universal/Network/SnpDxe/Snp.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> > b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> > index 9f61aee..f0257a2 100644
> > --- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> > +++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
> > @@ -1,9 +1,9 @@
> >  /** @file
> >    Implementation of driver entry point and driver binding protocol.
> >
> > -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2004 - 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
> >  http://opensource.org/licenses/bsd-license.php
> >
> > @@ -333,10 +333,15 @@ SimpleNetworkDriverStart (
> >      return Status;
> >    }
> >
> >    DEBUG ((EFI_D_INFO, "Start(): UNDI3.1 found\n"));
> >
> > +  if (Nii->Id == 0) {
> > +    DEBUG ((EFI_D_NET, "\nUNDI is not started.\n"));
> > +    goto NiiError;
> > +  }
> > +
> >    Pxe = (PXE_UNDI *) (UINTN) (Nii->Id);
> >
> >    if (Calc8BitCksum (Pxe, Pxe->hw.Len) != 0) {
> >      DEBUG ((EFI_D_NET, "\n!PXE checksum is not correct.\n"));
> >      goto NiiError;
> > --
> > 1.9.5.msysgit.1