public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured
@ 2016-08-15  2:46 Jiaxin Wu
  2016-08-15 14:20 ` Cohen, Eugene
  2016-08-16  7:57 ` Ye, Ting
  0 siblings, 2 replies; 3+ messages in thread
From: Jiaxin Wu @ 2016-08-15  2:46 UTC (permalink / raw)
  To: edk2-devel; +Cc: Cohen Eugene, Ye Ting, Fu Siyuan

IpSb->Reconfig should not be set to TRUE to focal the reconfiguration
during the policy changes from Static to DHCP. It's redundancy because
the default router table and default addresses have been freed ahead (
Detailed see Ip4Config2OnPolicyChanged() function). Otherwise, the
potential failure will appear if UseDefaultAddress configured. Reproduce
steps see below:

#1. Set policy to DHCP.
#2. If DHCP process is not complete yet, then run one APP to invoke UDP4 
Configure with "UseDefaultAddress = TRUE" (loop to call UDP4 Configure 
until Ip4Mode.IsConfigured changes to TRUE).
#3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to TRUE

Concrete analysis is as follows:
In #1, the policy will be set to DHCP, and then Ip4Config2OnPolicyChanged()
will be called. In this function, if "IpSb->Reconfig" flag is set to TRUE,
the original "IpSb->DefaultInterface" will be abandoned/freed once the
DHCP process finished.

In #2, UDP4 Configure with "UseDefaultAddress = TRUE" is called, that means
the default interface (IpSb->DefaultInterface) will be selected as current
instance's interface.

In #3, when DHCP process finished, the original DefaultInterface will be
abandoned/freed because "IpSb->Reconfig" flag is true. Meanwhile, one new
interface is assigned to "IpSb->DefaultInterface". This new interface is
different to the original one assigned to the UDP4 Configured instance. So,
even DHCP process succeed, the up caller will never have the chance to get
it's truly status.

Cc: Cohen Eugene <eugene@hp.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: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index f91a935..75ad301 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -148,11 +148,10 @@ Ip4Config2OnPolicyChanged (
 
   //
   // Start the dhcp configuration.
   //
   if (NewPolicy == Ip4Config2PolicyDhcp) {
-    IpSb->Reconfig = TRUE;
     Ip4StartAutoConfig (&IpSb->Ip4Config2Instance);
   }
 
 }
 
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured
  2016-08-15  2:46 [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured Jiaxin Wu
@ 2016-08-15 14:20 ` Cohen, Eugene
  2016-08-16  7:57 ` Ye, Ting
  1 sibling, 0 replies; 3+ messages in thread
From: Cohen, Eugene @ 2016-08-15 14:20 UTC (permalink / raw)
  To: Jiaxin Wu, edk2-devel@lists.01.org; +Cc: Ye Ting, Fu Siyuan

Tested-by: Eugene Cohen <eugene@hp.com>

with the provision that Configure() retries are maintained for TCP cases

> -----Original Message-----
> From: Jiaxin Wu [mailto:jiaxin.wu@intel.com]
> Sent: Sunday, August 14, 2016 8:47 PM
> To: edk2-devel@lists.01.org
> Cc: Cohen, Eugene <eugene@hp.com>; Ye Ting <ting.ye@intel.com>;
> Fu Siyuan <siyuan.fu@intel.com>
> Subject: [Patch] MdeModulePkg: Fix potential failure if
> UseDefaultAddress configured
> 
> IpSb->Reconfig should not be set to TRUE to focal the reconfiguration
> during the policy changes from Static to DHCP. It's redundancy because
> the default router table and default addresses have been freed ahead
> (
> Detailed see Ip4Config2OnPolicyChanged() function). Otherwise, the
> potential failure will appear if UseDefaultAddress configured.
> Reproduce
> steps see below:
> 
> #1. Set policy to DHCP.
> #2. If DHCP process is not complete yet, then run one APP to invoke
> UDP4
> Configure with "UseDefaultAddress = TRUE" (loop to call UDP4
> Configure
> until Ip4Mode.IsConfigured changes to TRUE).
> #3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to
> TRUE
> 
> Concrete analysis is as follows:
> In #1, the policy will be set to DHCP, and then
> Ip4Config2OnPolicyChanged()
> will be called. In this function, if "IpSb->Reconfig" flag is set to TRUE,
> the original "IpSb->DefaultInterface" will be abandoned/freed once
> the
> DHCP process finished.
> 
> In #2, UDP4 Configure with "UseDefaultAddress = TRUE" is called, that
> means
> the default interface (IpSb->DefaultInterface) will be selected as
> current
> instance's interface.
> 
> In #3, when DHCP process finished, the original DefaultInterface will be
> abandoned/freed because "IpSb->Reconfig" flag is true. Meanwhile,
> one new
> interface is assigned to "IpSb->DefaultInterface". This new interface is
> different to the original one assigned to the UDP4 Configured instance.
> So,
> even DHCP process succeed, the up caller will never have the chance
> to get
> it's truly status.
> 
> Cc: Cohen Eugene <eugene@hp.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: Jiaxin Wu <jiaxin.wu@intel.com>
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git
> a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index f91a935..75ad301 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -148,11 +148,10 @@ Ip4Config2OnPolicyChanged (
> 
>    //
>    // Start the dhcp configuration.
>    //
>    if (NewPolicy == Ip4Config2PolicyDhcp) {
> -    IpSb->Reconfig = TRUE;
>      Ip4StartAutoConfig (&IpSb->Ip4Config2Instance);
>    }
> 
>  }
> 
> --
> 1.9.5.msysgit.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured
  2016-08-15  2:46 [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured Jiaxin Wu
  2016-08-15 14:20 ` Cohen, Eugene
@ 2016-08-16  7:57 ` Ye, Ting
  1 sibling, 0 replies; 3+ messages in thread
From: Ye, Ting @ 2016-08-16  7:57 UTC (permalink / raw)
  To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Fu, Siyuan, Cohen Eugene

Looks good to me.
Reviewed-by: Ye Ting <ting.ye@intel.com> 

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Monday, August 15, 2016 10:47 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Cohen Eugene <eugene@hp.com>
Subject: [edk2] [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured

IpSb->Reconfig should not be set to TRUE to focal the reconfiguration
during the policy changes from Static to DHCP. It's redundancy because the default router table and default addresses have been freed ahead ( Detailed see Ip4Config2OnPolicyChanged() function). Otherwise, the potential failure will appear if UseDefaultAddress configured. Reproduce steps see below:

#1. Set policy to DHCP.
#2. If DHCP process is not complete yet, then run one APP to invoke UDP4 Configure with "UseDefaultAddress = TRUE" (loop to call UDP4 Configure until Ip4Mode.IsConfigured changes to TRUE).
#3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to TRUE

Concrete analysis is as follows:
In #1, the policy will be set to DHCP, and then Ip4Config2OnPolicyChanged() will be called. In this function, if "IpSb->Reconfig" flag is set to TRUE, the original "IpSb->DefaultInterface" will be abandoned/freed once the DHCP process finished.

In #2, UDP4 Configure with "UseDefaultAddress = TRUE" is called, that means the default interface (IpSb->DefaultInterface) will be selected as current instance's interface.

In #3, when DHCP process finished, the original DefaultInterface will be abandoned/freed because "IpSb->Reconfig" flag is true. Meanwhile, one new interface is assigned to "IpSb->DefaultInterface". This new interface is different to the original one assigned to the UDP4 Configured instance. So, even DHCP process succeed, the up caller will never have the chance to get it's truly status.

Cc: Cohen Eugene <eugene@hp.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: Jiaxin Wu <jiaxin.wu@intel.com>
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index f91a935..75ad301 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -148,11 +148,10 @@ Ip4Config2OnPolicyChanged (
 
   //
   // Start the dhcp configuration.
   //
   if (NewPolicy == Ip4Config2PolicyDhcp) {
-    IpSb->Reconfig = TRUE;
     Ip4StartAutoConfig (&IpSb->Ip4Config2Instance);
   }
 
 }
 
--
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-08-16  7:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-15  2:46 [Patch] MdeModulePkg: Fix potential failure if UseDefaultAddress configured Jiaxin Wu
2016-08-15 14:20 ` Cohen, Eugene
2016-08-16  7:57 ` Ye, Ting

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox