public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
@ 2016-10-06 21:40 Vladimir Olovyannikov
  2016-10-06 21:51 ` Shah, Tapan
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Olovyannikov @ 2016-10-06 21:40 UTC (permalink / raw)
  To: edk2-devel, tapandshah, jaben.carsey
  Cc: Vladimir Olovyannikov, Vladimir Olovyannikov

From: Vladimir Olovyannikov <volovyan@broadcom.com>

In ShellOpenFileByName() the file is opened using
gEfiShellProtocol->OpenFileByName().
It is supposed that if this call returns an EFI_ERROR, the function
should return that error immediately. However, this return was missing,
and if UnicodeCollationProtocol has not been located by this time, the
Status gets overwritten with LocateProtocol() call result, which
eventually erroneously returns EFI_SUCCESS to the Shell.c, and this
leads to attempt to execute a non-existent startup script, which fails,
and which in turn leads to Shell being unloaded with "Invalid parameter"
error. This patch fixes the bug.

Cc: Tapan Shah <tapandshah@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 53f54e1746d4..6fa67a3e751c 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -723,6 +723,8 @@ ShellOpenFileByName(
     Status = gEfiShellProtocol->OpenFileByName(FileName,
                                                FileHandle,
                                                OpenMode);
+    if (EFI_ERROR(Status)) {
+      return Status;
 
     if (mUnicodeCollationProtocol == NULL) {
       Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol);
-- 
1.9.1



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

* Re: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
  2016-10-06 21:40 [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName() Vladimir Olovyannikov
@ 2016-10-06 21:51 ` Shah, Tapan
  2016-10-06 21:52   ` Vladimir Olovyannikov
  0 siblings, 1 reply; 6+ messages in thread
From: Shah, Tapan @ 2016-10-06 21:51 UTC (permalink / raw)
  To: Vladimir Olovyannikov, edk2-devel@lists.01.org,
	jaben.carsey@intel.com
  Cc: Vladimir Olovyannikov

+    if (EFI_ERROR(Status)) {
+      return Status;
+    } <<<<<<<<<<<< Missing closing brace

Reviewed-by: Tapan Shah <tapandshah@hpe.com>

-----Original Message-----
From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com] 
Sent: Thursday, October 06, 2016 4:40 PM
To: edk2-devel@lists.01.org; Shah, Tapan <tapandshah@hpe.com>; jaben.carsey@intel.com
Cc: Vladimir Olovyannikov <volovyan@broadcom.com>; Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Subject: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()

From: Vladimir Olovyannikov <volovyan@broadcom.com>

In ShellOpenFileByName() the file is opened using
gEfiShellProtocol->OpenFileByName().
It is supposed that if this call returns an EFI_ERROR, the function should return that error immediately. However, this return was missing, and if UnicodeCollationProtocol has not been located by this time, the Status gets overwritten with LocateProtocol() call result, which eventually erroneously returns EFI_SUCCESS to the Shell.c, and this leads to attempt to execute a non-existent startup script, which fails, and which in turn leads to Shell being unloaded with "Invalid parameter"
error. This patch fixes the bug.

Cc: Tapan Shah <tapandshah@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 53f54e1746d4..6fa67a3e751c 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -723,6 +723,8 @@ ShellOpenFileByName(
     Status = gEfiShellProtocol->OpenFileByName(FileName,
                                                FileHandle,
                                                OpenMode);
+    if (EFI_ERROR(Status)) {
+      return Status;
 
     if (mUnicodeCollationProtocol == NULL) {
       Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol);
--
1.9.1



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

* Re: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
  2016-10-06 21:51 ` Shah, Tapan
@ 2016-10-06 21:52   ` Vladimir Olovyannikov
  0 siblings, 0 replies; 6+ messages in thread
From: Vladimir Olovyannikov @ 2016-10-06 21:52 UTC (permalink / raw)
  To: Shah, Tapan, edk2-devel, jaben.carsey; +Cc: Vladimir Olovyannikov

-----Original Message-----
From: Shah, Tapan [mailto:tapandshah@hpe.com]
Sent: Thursday, October 06, 2016 2:52 PM
To: Vladimir Olovyannikov; edk2-devel@lists.01.org; jaben.carsey@intel.com
Cc: Vladimir Olovyannikov
Subject: RE: [PATCH] ShellPkg: Fix erroneous Status returned by
ShellOpenFileByName()

+    if (EFI_ERROR(Status)) {
+      return Status;
+    } <<<<<<<<<<<< Missing closing brace


Right. Please disregard this, will resubmit.

Reviewed-by: Tapan Shah <tapandshah@hpe.com>

-----Original Message-----
From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com]
Sent: Thursday, October 06, 2016 4:40 PM
To: edk2-devel@lists.01.org; Shah, Tapan <tapandshah@hpe.com>;
jaben.carsey@intel.com
Cc: Vladimir Olovyannikov <volovyan@broadcom.com>; Vladimir Olovyannikov
<vladimir.olovyannikov@broadcom.com>
Subject: [PATCH] ShellPkg: Fix erroneous Status returned by
ShellOpenFileByName()

From: Vladimir Olovyannikov <volovyan@broadcom.com>

In ShellOpenFileByName() the file is opened using
gEfiShellProtocol->OpenFileByName().
It is supposed that if this call returns an EFI_ERROR, the function should
return that error immediately. However, this return was missing, and if
UnicodeCollationProtocol has not been located by this time, the Status
gets overwritten with LocateProtocol() call result, which eventually
erroneously returns EFI_SUCCESS to the Shell.c, and this leads to attempt
to execute a non-existent startup script, which fails, and which in turn
leads to Shell being unloaded with "Invalid parameter"
error. This patch fixes the bug.

Cc: Tapan Shah <tapandshah@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 53f54e1746d4..6fa67a3e751c 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -723,6 +723,8 @@ ShellOpenFileByName(
     Status = gEfiShellProtocol->OpenFileByName(FileName,
                                                FileHandle,
                                                OpenMode);
+    if (EFI_ERROR(Status)) {
+      return Status;

     if (mUnicodeCollationProtocol == NULL) {
       Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
NULL, (VOID**)&mUnicodeCollationProtocol);
--
1.9.1


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

* [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
@ 2016-10-06 22:02 Vladimir Olovyannikov
  2016-10-06 22:03 ` Shah, Tapan
  0 siblings, 1 reply; 6+ messages in thread
From: Vladimir Olovyannikov @ 2016-10-06 22:02 UTC (permalink / raw)
  To: edk2-devel, tapandshah, jaben.carsey; +Cc: Vladimir Olovyannikov

In ShellOpenFileByName() the file is opened using
gEfiShellProtocol->OpenFileByName().
It is supposed that if this call returns an EFI_ERROR, the function
should return that error immediately. However, this return was missing,
and if UnicodeCollationProtocol has not been located by this time, the
Status gets overwritten with LocateProtocol() call result, which
eventually erroneously returns EFI_SUCCESS to the Shell.c, and this
leads to attempt to execute a non-existent startup script, which fails,
and which in turn leads to Shell being unloaded with "Invalid parameter"
error. This patch fixes the bug.

Cc: Tapan Shah <tapandshah@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 53f54e1746d4..8db18b3b210f 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -723,6 +723,9 @@ ShellOpenFileByName(
     Status = gEfiShellProtocol->OpenFileByName(FileName,
                                                FileHandle,
                                                OpenMode);
+    if (EFI_ERROR(Status)) {
+      return Status;
+    }
 
     if (mUnicodeCollationProtocol == NULL) {
       Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol);
-- 
1.9.1



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

* Re: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
  2016-10-06 22:02 Vladimir Olovyannikov
@ 2016-10-06 22:03 ` Shah, Tapan
  2016-10-07 17:36   ` Carsey, Jaben
  0 siblings, 1 reply; 6+ messages in thread
From: Shah, Tapan @ 2016-10-06 22:03 UTC (permalink / raw)
  To: Vladimir Olovyannikov, edk2-devel@lists.01.org,
	jaben.carsey@intel.com

Reviewed-by: Tapan Shah <tapandshah@hpe.com>


-----Original Message-----
From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com] 
Sent: Thursday, October 06, 2016 5:02 PM
To: edk2-devel@lists.01.org; Shah, Tapan <tapandshah@hpe.com>; jaben.carsey@intel.com
Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
Subject: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()

In ShellOpenFileByName() the file is opened using
gEfiShellProtocol->OpenFileByName().
It is supposed that if this call returns an EFI_ERROR, the function should return that error immediately. However, this return was missing, and if UnicodeCollationProtocol has not been located by this time, the Status gets overwritten with LocateProtocol() call result, which eventually erroneously returns EFI_SUCCESS to the Shell.c, and this leads to attempt to execute a non-existent startup script, which fails, and which in turn leads to Shell being unloaded with "Invalid parameter"
error. This patch fixes the bug.

Cc: Tapan Shah <tapandshah@hpe.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index 53f54e1746d4..8db18b3b210f 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -723,6 +723,9 @@ ShellOpenFileByName(
     Status = gEfiShellProtocol->OpenFileByName(FileName,
                                                FileHandle,
                                                OpenMode);
+    if (EFI_ERROR(Status)) {
+      return Status;
+    }
 
     if (mUnicodeCollationProtocol == NULL) {
       Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, (VOID**)&mUnicodeCollationProtocol);
--
1.9.1



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

* Re: [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName()
  2016-10-06 22:03 ` Shah, Tapan
@ 2016-10-07 17:36   ` Carsey, Jaben
  0 siblings, 0 replies; 6+ messages in thread
From: Carsey, Jaben @ 2016-10-07 17:36 UTC (permalink / raw)
  To: Shah, Tapan, Vladimir Olovyannikov, edk2-devel@lists.01.org; +Cc: Carsey, Jaben

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
And pushed.

> -----Original Message-----
> From: Shah, Tapan [mailto:tapandshah@hpe.com]
> Sent: Thursday, October 06, 2016 3:04 PM
> To: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>; edk2-
> devel@lists.01.org; Carsey, Jaben <jaben.carsey@intel.com>
> Subject: RE: [PATCH] ShellPkg: Fix erroneous Status returned by
> ShellOpenFileByName()
> Importance: High
> 
> Reviewed-by: Tapan Shah <tapandshah@hpe.com>
> 
> 
> -----Original Message-----
> From: Vladimir Olovyannikov [mailto:vladimir.olovyannikov@broadcom.com]
> Sent: Thursday, October 06, 2016 5:02 PM
> To: edk2-devel@lists.01.org; Shah, Tapan <tapandshah@hpe.com>;
> jaben.carsey@intel.com
> Cc: Vladimir Olovyannikov <vladimir.olovyannikov@broadcom.com>
> Subject: [PATCH] ShellPkg: Fix erroneous Status returned by
> ShellOpenFileByName()
> 
> In ShellOpenFileByName() the file is opened using
> gEfiShellProtocol->OpenFileByName().
> It is supposed that if this call returns an EFI_ERROR, the function should
> return that error immediately. However, this return was missing, and if
> UnicodeCollationProtocol has not been located by this time, the Status gets
> overwritten with LocateProtocol() call result, which eventually erroneously
> returns EFI_SUCCESS to the Shell.c, and this leads to attempt to execute a
> non-existent startup script, which fails, and which in turn leads to Shell being
> unloaded with "Invalid parameter"
> error. This patch fixes the bug.
> 
> Cc: Tapan Shah <tapandshah@hpe.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Vladimir Olovyannikov
> <vladimir.olovyannikov@broadcom.com>
> ---
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index 53f54e1746d4..8db18b3b210f 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -723,6 +723,9 @@ ShellOpenFileByName(
>      Status = gEfiShellProtocol->OpenFileByName(FileName,
>                                                 FileHandle,
>                                                 OpenMode);
> +    if (EFI_ERROR(Status)) {
> +      return Status;
> +    }
> 
>      if (mUnicodeCollationProtocol == NULL) {
>        Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
> NULL, (VOID**)&mUnicodeCollationProtocol);
> --
> 1.9.1
> 



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

end of thread, other threads:[~2016-10-07 17:36 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-06 21:40 [PATCH] ShellPkg: Fix erroneous Status returned by ShellOpenFileByName() Vladimir Olovyannikov
2016-10-06 21:51 ` Shah, Tapan
2016-10-06 21:52   ` Vladimir Olovyannikov
  -- strict thread matches above, loose matches on Subject: below --
2016-10-06 22:02 Vladimir Olovyannikov
2016-10-06 22:03 ` Shah, Tapan
2016-10-07 17:36   ` Carsey, Jaben

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