public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
@ 2018-09-14  8:24 Wang Fan
  2018-09-14  8:50 ` Subramanian, Sriram
  0 siblings, 1 reply; 7+ messages in thread
From: Wang Fan @ 2018-09-14  8:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: Subramanian, Sriram, Ye Ting, Fu Siyuan, Wu Jiaxin

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152

v2: Sync the same logic to Ipv6 and update code comments.

The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
to avoid it being opened by MNP driver, this SNP is also expected not to
be opened by other drivers with EXCLUSIVE attribute. In some cases, other
drivers may happen to do this by error, and thus cause a system crash.
This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
will reject all OpenProtocol requests by EXCLUSIVE.

Cc: Subramanian, Sriram <sriram-s@hpe.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
index bc9dc914f3..0ab640beca 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
@@ -812,20 +812,20 @@ PxeBcCreateIp4Children (
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
 
     //
-    // Open SNP on the child handle BY_DRIVER. It will prevent any additionally
+    // Open SNP on the child handle BY_DRIVER|EXCLUSIVE. It will prevent any additionally
     // layering to perform the experiment.
     //
     Status = gBS->OpenProtocol (
                     Private->Ip4Nic->Controller,
                     &gEfiSimpleNetworkProtocolGuid,
                     (VOID **) &Snp,
                     This->DriverBindingHandle,
                     Private->Ip4Nic->Controller,
-                    EFI_OPEN_PROTOCOL_BY_DRIVER
+                    EFI_OPEN_PROTOCOL_BY_DRIVER|EFI_OPEN_PROTOCOL_EXCLUSIVE
                     );
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
   }
@@ -1155,20 +1155,20 @@ PxeBcCreateIp6Children (
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
 
     //
-    // Open SNP on the child handle BY_DRIVER. It will prevent any additionally
+    // Open SNP on the child handle BY_DRIVER|EXCLUSIVE. It will prevent any additionally
     // layering to perform the experiment.
     //
     Status = gBS->OpenProtocol (
                     Private->Ip6Nic->Controller,
                     &gEfiSimpleNetworkProtocolGuid,
                     (VOID **) &Snp,
                     This->DriverBindingHandle,
                     Private->Ip6Nic->Controller,
-                    EFI_OPEN_PROTOCOL_BY_DRIVER
+                    EFI_OPEN_PROTOCOL_BY_DRIVER|EFI_OPEN_PROTOCOL_EXCLUSIVE
                     );
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
   }
-- 
2.16.2.windows.1



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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-14  8:24 [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE Wang Fan
@ 2018-09-14  8:50 ` Subramanian, Sriram
  2018-09-18 10:17   ` Laszlo Ersek
  0 siblings, 1 reply; 7+ messages in thread
From: Subramanian, Sriram @ 2018-09-14  8:50 UTC (permalink / raw)
  To: Wang Fan, edk2-devel@lists.01.org
  Cc: Subramanian@mx0b-002e3701.pphosted.com, Ye Ting, Fu Siyuan,
	Wu Jiaxin

Reviewed-by: Sriram Subramanian <sriram-s@hpe.com>

-----Original Message-----
From: Wang Fan [mailto:fan.wang@intel.com] 
Sent: Friday, September 14, 2018 1:54 PM
To: edk2-devel@lists.01.org
Cc: Subramanian@mx0b-002e3701.pphosted.com; Subramanian, Sriram <sriram-s@hpe.com>; Ye Ting <ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin <jiaxin.wu@intel.com>
Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152

v2: Sync the same logic to Ipv6 and update code comments.

The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
to avoid it being opened by MNP driver, this SNP is also expected not to
be opened by other drivers with EXCLUSIVE attribute. In some cases, other
drivers may happen to do this by error, and thus cause a system crash.
This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
will reject all OpenProtocol requests by EXCLUSIVE.

Cc: Subramanian, Sriram <sriram-s@hpe.com>
Cc: Ye Ting <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Cc: Wu Jiaxin <jiaxin.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wang Fan <fan.wang@intel.com>
---
 NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
index bc9dc914f3..0ab640beca 100644
--- a/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
+++ b/NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c
@@ -812,20 +812,20 @@ PxeBcCreateIp4Children (
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
 
     //
-    // Open SNP on the child handle BY_DRIVER. It will prevent any additionally
+    // Open SNP on the child handle BY_DRIVER|EXCLUSIVE. It will prevent any additionally
     // layering to perform the experiment.
     //
     Status = gBS->OpenProtocol (
                     Private->Ip4Nic->Controller,
                     &gEfiSimpleNetworkProtocolGuid,
                     (VOID **) &Snp,
                     This->DriverBindingHandle,
                     Private->Ip4Nic->Controller,
-                    EFI_OPEN_PROTOCOL_BY_DRIVER
+                    EFI_OPEN_PROTOCOL_BY_DRIVER|EFI_OPEN_PROTOCOL_EXCLUSIVE
                     );
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
   }
@@ -1155,20 +1155,20 @@ PxeBcCreateIp6Children (
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
 
     //
-    // Open SNP on the child handle BY_DRIVER. It will prevent any additionally
+    // Open SNP on the child handle BY_DRIVER|EXCLUSIVE. It will prevent any additionally
     // layering to perform the experiment.
     //
     Status = gBS->OpenProtocol (
                     Private->Ip6Nic->Controller,
                     &gEfiSimpleNetworkProtocolGuid,
                     (VOID **) &Snp,
                     This->DriverBindingHandle,
                     Private->Ip6Nic->Controller,
-                    EFI_OPEN_PROTOCOL_BY_DRIVER
+                    EFI_OPEN_PROTOCOL_BY_DRIVER|EFI_OPEN_PROTOCOL_EXCLUSIVE
                     );
     if (EFI_ERROR (Status)) {
       goto ON_ERROR;
     }
   }
-- 
2.16.2.windows.1



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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-14  8:50 ` Subramanian, Sriram
@ 2018-09-18 10:17   ` Laszlo Ersek
  2018-09-19  1:31     ` Wu, Jiaxin
  0 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2018-09-18 10:17 UTC (permalink / raw)
  To: Subramanian, Sriram, Wang Fan, edk2-devel@lists.01.org
  Cc: Ye Ting, Fu Siyuan, Wu Jiaxin,
	Subramanian@mx0b-002e3701.pphosted.com

On 09/14/18 10:50, Subramanian, Sriram wrote:
> Reviewed-by: Sriram Subramanian <sriram-s@hpe.com>
> 
> -----Original Message-----
> From: Wang Fan [mailto:fan.wang@intel.com] 
> Sent: Friday, September 14, 2018 1:54 PM
> To: edk2-devel@lists.01.org
> Cc: Subramanian@mx0b-002e3701.pphosted.com; Subramanian, Sriram <sriram-s@hpe.com>; Ye Ting <ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> 
> v2: Sync the same logic to Ipv6 and update code comments.
> 
> The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
> to avoid it being opened by MNP driver, this SNP is also expected not to
> be opened by other drivers with EXCLUSIVE attribute. In some cases, other
> drivers may happen to do this by error, and thus cause a system crash.
> This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
> will reject all OpenProtocol requests by EXCLUSIVE.
> 
> Cc: Subramanian, Sriram <sriram-s@hpe.com>
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wang Fan <fan.wang@intel.com>
> ---
>  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Either my edk2-devel subscription is breaking down, or our discipline
regarding commit pushing is down the drain, recently.

This patch has been pushed as commit cde5a72d365e. Problems with that
commit:

(1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
That's *wrong*. This patch was authored by Wang Fan <fan.wang@intel.com>.

(2) The commit lists Siyuan and Jiaxin as having given their reviews. I
don't see their *v2* reviews anywhere on the list. I see their *v1*
reviews, but the patch was changed in v2. Wang Fan was correct not to
carry forward the v1 reviews into the v2 posting. If the v2 posting was
fine, then Siyuan and Jiaxin should have confirmed that (with R-b's) on
the list, before pushing the commit.

Another process failure that I'm witnessing is that people forget to
close BZs once the corresponding fixes are upstream. Sorry but that just
makes a joke out of bug tracking.

Come on, guys. We can do better than this. Show some discipline and
dedication to the process, please. The process is not self-serving; it's
in place to promote mutual understanding.

Laszlo


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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-18 10:17   ` Laszlo Ersek
@ 2018-09-19  1:31     ` Wu, Jiaxin
  2018-09-19  1:44       ` Wu, Jiaxin
  2018-09-19 11:19       ` Laszlo Ersek
  0 siblings, 2 replies; 7+ messages in thread
From: Wu, Jiaxin @ 2018-09-19  1:31 UTC (permalink / raw)
  To: Laszlo Ersek, Subramanian, Sriram, Wang, Fan,
	edk2-devel@lists.01.org
  Cc: Ye, Ting, Fu, Siyuan, Subramanian@mx0b-002e3701.pphosted.com

> > Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute
> when opening SNP protocol installed by PXE.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> >
> > v2: Sync the same logic to Ipv6 and update code comments.
> >
> > The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
> > to avoid it being opened by MNP driver, this SNP is also expected not to
> > be opened by other drivers with EXCLUSIVE attribute. In some cases, other
> > drivers may happen to do this by error, and thus cause a system crash.
> > This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
> > will reject all OpenProtocol requests by EXCLUSIVE.
> >
> > Cc: Subramanian, Sriram <sriram-s@hpe.com>
> > Cc: Ye Ting <ting.ye@intel.com>
> > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > ---
> >  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> Either my edk2-devel subscription is breaking down, or our discipline
> regarding commit pushing is down the drain, recently.
> 
> This patch has been pushed as commit cde5a72d365e. Problems with that
> commit:
> 
> (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
> That's *wrong*. This patch was authored by Wang Fan
> <fan.wang@intel.com>.
> 

Sorry Laszlo, I didn't check the "Author" field before pushing the patch as I received the commit request form Fan. I don't know whether it's the patch reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field. Anyway, I will double check that. 

> (2) The commit lists Siyuan and Jiaxin as having given their reviews. I
> don't see their *v2* reviews anywhere on the list. I see their *v1*
> reviews, but the patch was changed in v2. Wang Fan was correct not to
> carry forward the v1 reviews into the v2 posting. If the v2 posting was
> fine, then Siyuan and Jiaxin should have confirmed that (with R-b's) on
> the list, before pushing the commit.
> 

Yes, you are right, Fan should include the v1 reviewer into the v2 posting, and the reviewer should confirm the new version patch even he/she is the final version committer.

> Another process failure that I'm witnessing is that people forget to
> close BZs once the corresponding fixes are upstream. Sorry but that just
> makes a joke out of bug tracking.
> 

Thanks the reminder, we will pay attention next time.

> Come on, guys. We can do better than this. Show some discipline and
> dedication to the process, please. The process is not self-serving; it's
> in place to promote mutual understanding.
> 
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-19  1:31     ` Wu, Jiaxin
@ 2018-09-19  1:44       ` Wu, Jiaxin
  2018-09-19  1:46         ` Wang, Fan
  2018-09-19 11:19       ` Laszlo Ersek
  1 sibling, 1 reply; 7+ messages in thread
From: Wu, Jiaxin @ 2018-09-19  1:44 UTC (permalink / raw)
  To: Wu, Jiaxin, Laszlo Ersek, Subramanian, Sriram, Wang, Fan,
	edk2-devel@lists.01.org
  Cc: Ye, Ting, Fu, Siyuan, Subramanian@mx0b-002e3701.pphosted.com

Correct one of my comments.

The new version patch should *NOT* include the reviewed-by tag from previous version.

Thanks,
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Wu, Jiaxin
> Sent: Wednesday, September 19, 2018 9:32 AM
> To: Laszlo Ersek <lersek@redhat.com>; Subramanian, Sriram <sriram-
> s@hpe.com>; Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> Subramanian@mx0b-002e3701.pphosted.com
> Subject: Re: [edk2] [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE
> attribute when opening SNP protocol installed by PXE.
> 
> > > Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute
> > when opening SNP protocol installed by PXE.
> > >
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> > >
> > > v2: Sync the same logic to Ipv6 and update code comments.
> > >
> > > The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
> > > to avoid it being opened by MNP driver, this SNP is also expected not to
> > > be opened by other drivers with EXCLUSIVE attribute. In some cases,
> other
> > > drivers may happen to do this by error, and thus cause a system crash.
> > > This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
> > > will reject all OpenProtocol requests by EXCLUSIVE.
> > >
> > > Cc: Subramanian, Sriram <sriram-s@hpe.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > > ---
> > >  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > Either my edk2-devel subscription is breaking down, or our discipline
> > regarding commit pushing is down the drain, recently.
> >
> > This patch has been pushed as commit cde5a72d365e. Problems with that
> > commit:
> >
> > (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
> > That's *wrong*. This patch was authored by Wang Fan
> > <fan.wang@intel.com>.
> >
> 
> Sorry Laszlo, I didn't check the "Author" field before pushing the patch as I
> received the commit request form Fan. I don't know whether it's the patch
> reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field.
> Anyway, I will double check that.
> 
> > (2) The commit lists Siyuan and Jiaxin as having given their reviews. I
> > don't see their *v2* reviews anywhere on the list. I see their *v1*
> > reviews, but the patch was changed in v2. Wang Fan was correct not to
> > carry forward the v1 reviews into the v2 posting. If the v2 posting was
> > fine, then Siyuan and Jiaxin should have confirmed that (with R-b's) on
> > the list, before pushing the commit.
> >
> 
> Yes, you are right, Fan should include the v1 reviewer into the v2 posting, and
> the reviewer should confirm the new version patch even he/she is the final
> version committer.
> 


The new version patch should *NOT* include the reviewed-by tag from previous version. 
 


> > Another process failure that I'm witnessing is that people forget to
> > close BZs once the corresponding fixes are upstream. Sorry but that just
> > makes a joke out of bug tracking.
> >
> 
> Thanks the reminder, we will pay attention next time.
> 
> > Come on, guys. We can do better than this. Show some discipline and
> > dedication to the process, please. The process is not self-serving; it's
> > in place to promote mutual understanding.
> >
> > Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-19  1:44       ` Wu, Jiaxin
@ 2018-09-19  1:46         ` Wang, Fan
  0 siblings, 0 replies; 7+ messages in thread
From: Wang, Fan @ 2018-09-19  1:46 UTC (permalink / raw)
  To: Wu, Jiaxin, Laszlo Ersek, Subramanian, Sriram,
	edk2-devel@lists.01.org
  Cc: Ye, Ting, Fu, Siyuan, Subramanian@mx0b-002e3701.pphosted.com

Thanks, Laszlo and Jiaxin, I will have a check about the author name "edk2-devel-bounces@lists.01.org".

Best Regards
Fan

-----Original Message-----
From: Wu, Jiaxin 
Sent: Wednesday, September 19, 2018 9:44 AM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>; Subramanian, Sriram <sriram-s@hpe.com>; Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Subramanian@mx0b-002e3701.pphosted.com
Subject: RE: [edk2] [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.

Correct one of my comments.

The new version patch should *NOT* include the reviewed-by tag from previous version.

Thanks,
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Wu, Jiaxin
> Sent: Wednesday, September 19, 2018 9:32 AM
> To: Laszlo Ersek <lersek@redhat.com>; Subramanian, Sriram <sriram- 
> s@hpe.com>; Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; 
> Subramanian@mx0b-002e3701.pphosted.com
> Subject: Re: [edk2] [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE 
> attribute when opening SNP protocol installed by PXE.
> 
> > > Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE 
> > > attribute
> > when opening SNP protocol installed by PXE.
> > >
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> > >
> > > v2: Sync the same logic to Ipv6 and update code comments.
> > >
> > > The PXE driver installs a SNP and open this SNP with attribute 
> > > BY_DRIVER to avoid it being opened by MNP driver, this SNP is also 
> > > expected not to be opened by other drivers with EXCLUSIVE 
> > > attribute. In some cases,
> other
> > > drivers may happen to do this by error, and thus cause a system crash.
> > > This patch adds EXCLUSIVE attribute when opening SNP in PXE 
> > > driver, and will reject all OpenProtocol requests by EXCLUSIVE.
> > >
> > > Cc: Subramanian, Sriram <sriram-s@hpe.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > > ---
> > >  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > Either my edk2-devel subscription is breaking down, or our 
> > discipline regarding commit pushing is down the drain, recently.
> >
> > This patch has been pushed as commit cde5a72d365e. Problems with 
> > that
> > commit:
> >
> > (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
> > That's *wrong*. This patch was authored by Wang Fan 
> > <fan.wang@intel.com>.
> >
> 
> Sorry Laszlo, I didn't check the "Author" field before pushing the 
> patch as I received the commit request form Fan. I don't know whether 
> it's the patch reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field.
> Anyway, I will double check that.
> 
> > (2) The commit lists Siyuan and Jiaxin as having given their 
> > reviews. I don't see their *v2* reviews anywhere on the list. I see 
> > their *v1* reviews, but the patch was changed in v2. Wang Fan was 
> > correct not to carry forward the v1 reviews into the v2 posting. If 
> > the v2 posting was fine, then Siyuan and Jiaxin should have 
> > confirmed that (with R-b's) on the list, before pushing the commit.
> >
> 
> Yes, you are right, Fan should include the v1 reviewer into the v2 
> posting, and the reviewer should confirm the new version patch even 
> he/she is the final version committer.
> 


The new version patch should *NOT* include the reviewed-by tag from previous version. 
 


> > Another process failure that I'm witnessing is that people forget to 
> > close BZs once the corresponding fixes are upstream. Sorry but that 
> > just makes a joke out of bug tracking.
> >
> 
> Thanks the reminder, we will pay attention next time.
> 
> > Come on, guys. We can do better than this. Show some discipline and 
> > dedication to the process, please. The process is not self-serving; 
> > it's in place to promote mutual understanding.
> >
> > Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


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

* Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.
  2018-09-19  1:31     ` Wu, Jiaxin
  2018-09-19  1:44       ` Wu, Jiaxin
@ 2018-09-19 11:19       ` Laszlo Ersek
  1 sibling, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2018-09-19 11:19 UTC (permalink / raw)
  To: Wu, Jiaxin, Subramanian, Sriram, Wang, Fan,
	edk2-devel@lists.01.org
  Cc: Ye, Ting, Fu, Siyuan, Subramanian@mx0b-002e3701.pphosted.com

On 09/19/18 03:31, Wu, Jiaxin wrote:
>>> Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute
>> when opening SNP protocol installed by PXE.
>>>
>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
>>>
>>> v2: Sync the same logic to Ipv6 and update code comments.
>>>
>>> The PXE driver installs a SNP and open this SNP with attribute BY_DRIVER
>>> to avoid it being opened by MNP driver, this SNP is also expected not to
>>> be opened by other drivers with EXCLUSIVE attribute. In some cases, other
>>> drivers may happen to do this by error, and thus cause a system crash.
>>> This patch adds EXCLUSIVE attribute when opening SNP in PXE driver, and
>>> will reject all OpenProtocol requests by EXCLUSIVE.
>>>
>>> Cc: Subramanian, Sriram <sriram-s@hpe.com>
>>> Cc: Ye Ting <ting.ye@intel.com>
>>> Cc: Fu Siyuan <siyuan.fu@intel.com>
>>> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Wang Fan <fan.wang@intel.com>
>>> ---
>>>  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
>>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> Either my edk2-devel subscription is breaking down, or our discipline
>> regarding commit pushing is down the drain, recently.
>>
>> This patch has been pushed as commit cde5a72d365e. Problems with that
>> commit:
>>
>> (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
>> That's *wrong*. This patch was authored by Wang Fan
>> <fan.wang@intel.com>.
>>
> 
> Sorry Laszlo, I didn't check the "Author" field before pushing the patch as I received the commit request form Fan. I don't know whether it's the patch reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field. Anyway, I will double check that. 

Thanks. You can always correct the authorship metadata with:

git commit --amend --author='Name <email@example.com>'

without staging any code changes first. (So that the patch body is not
itself modified.)

Thanks!
Laszlo


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

end of thread, other threads:[~2018-09-19 11:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-14  8:24 [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE Wang Fan
2018-09-14  8:50 ` Subramanian, Sriram
2018-09-18 10:17   ` Laszlo Ersek
2018-09-19  1:31     ` Wu, Jiaxin
2018-09-19  1:44       ` Wu, Jiaxin
2018-09-19  1:46         ` Wang, Fan
2018-09-19 11:19       ` Laszlo Ersek

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