1. Patchew
  2. EDK2
  3. View series

[edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()

Hao Wu posted 1 patch 7 years, 1 month ago
Failed in applying to current master (apply log)
ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
[edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
Posted by Hao Wu 7 years, 1 month ago 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779

For the API EfiShellOpenRootByHandle():

The return status of the call to SimpleFileSystem->OpenVolume should be
checked.

It is possible that there is a media change in the device (like CD/DVD
ROM). In such case, the volume root opened and/or the device path opened
previously (also within EfiShellOpenRootByHandle) may be invalid.

This commit adds a check for the result of OpenVolume before subsequently
calling functions like EfiShellGetMapFromDevicePath() &
ConvertEfiFileProtocolToShellHandle().

Cc: Jaben Carsey <jaben.carsey@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Application/Shell/ShellProtocol.c
index 5e34b8dad1..0dee267353 100644
--- a/ShellPkg/Application/Shell/ShellProtocol.c
+++ b/ShellPkg/Application/Shell/ShellProtocol.c
@@ -827,7 +827,8 @@ EfiShellGetDeviceName(
   @retval EFI_NOT_FOUND         EFI_SIMPLE_FILE_SYSTEM could not be found or the root directory
                                 could not be opened.
   @retval EFI_VOLUME_CORRUPTED  The data structures in the volume were corrupted.
-  @retval EFI_DEVICE_ERROR      The device had an error
+  @retval EFI_DEVICE_ERROR      The device had an error.
+  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the medium is no longer supported.
 **/
 EFI_STATUS
 EFIAPI
@@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
   // Open the root volume now...
   //
   Status = SimpleFileSystem->OpenVolume(SimpleFileSystem, &RealFileHandle);
+  if (EFI_ERROR(Status)) {
+    return Status;
+  }
+
   *FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle, EfiShellGetMapFromDevicePath(&DevPath));
-  return (Status);
+  return (EFI_SUCCESS);
 }
 
 /**
-- 
2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
Posted by Ni, Ruiyu 7 years, 1 month ago 
How about changing the function header comments from
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or the medium is no longer supported.

To:
> +  @retval others  Error status returned from EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().

Because MEDIA_CHANGED is not the only error that could happen.


Thanks/Ray

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 3:54 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> 
> For the API EfiShellOpenRootByHandle():
> 
> The return status of the call to SimpleFileSystem->OpenVolume should be
> checked.
> 
> It is possible that there is a media change in the device (like CD/DVD ROM). In
> such case, the volume root opened and/or the device path opened previously
> (also within EfiShellOpenRootByHandle) may be invalid.
> 
> This commit adds a check for the result of OpenVolume before subsequently
> calling functions like EfiShellGetMapFromDevicePath() &
> ConvertEfiFileProtocolToShellHandle().
> 
> Cc: Jaben Carsey <jaben.carsey@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index 5e34b8dad1..0dee267353 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
>    @retval EFI_NOT_FOUND         EFI_SIMPLE_FILE_SYSTEM could not be found
> or the root directory
>                                  could not be opened.
>    @retval EFI_VOLUME_CORRUPTED  The data structures in the volume were
> corrupted.
> -  @retval EFI_DEVICE_ERROR      The device had an error
> +  @retval EFI_DEVICE_ERROR      The device had an error.
> +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it or
> the medium is no longer supported.
>  **/
>  EFI_STATUS
>  EFIAPI
> @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
>    // Open the root volume now...
>    //
>    Status = SimpleFileSystem->OpenVolume(SimpleFileSystem, &RealFileHandle);
> +  if (EFI_ERROR(Status)) {
> +    return Status;
> +  }
> +
>    *FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> EfiShellGetMapFromDevicePath(&DevPath));
> -  return (Status);
> +  return (EFI_SUCCESS);
>  }
> 
>  /**
> --
> 2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
Posted by Wu, Hao A 7 years, 1 month ago 
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, November 14, 2017 4:38 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Carsey, Jaben
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> How about changing the function header comments from
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or the medium is no longer supported.
> 
> To:
> > +  @retval others  Error status returned from
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().
> 
> Because MEDIA_CHANGED is not the only error that could happen.

Yes, I will update the comments when I push the patch if there's no other
comments.


Best Regards,
Hao Wu

> 
> 
> Thanks/Ray
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Tuesday, November 14, 2017 3:54 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
> > <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > OpenRootByHandle()
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> >
> > For the API EfiShellOpenRootByHandle():
> >
> > The return status of the call to SimpleFileSystem->OpenVolume should be
> > checked.
> >
> > It is possible that there is a media change in the device (like CD/DVD ROM).
> In
> > such case, the volume root opened and/or the device path opened previously
> > (also within EfiShellOpenRootByHandle) may be invalid.
> >
> > This commit adds a check for the result of OpenVolume before subsequently
> > calling functions like EfiShellGetMapFromDevicePath() &
> > ConvertEfiFileProtocolToShellHandle().
> >
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
> >  1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> > b/ShellPkg/Application/Shell/ShellProtocol.c
> > index 5e34b8dad1..0dee267353 100644
> > --- a/ShellPkg/Application/Shell/ShellProtocol.c
> > +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> > @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
> >    @retval EFI_NOT_FOUND         EFI_SIMPLE_FILE_SYSTEM could not be
> found
> > or the root directory
> >                                  could not be opened.
> >    @retval EFI_VOLUME_CORRUPTED  The data structures in the volume
> were
> > corrupted.
> > -  @retval EFI_DEVICE_ERROR      The device had an error
> > +  @retval EFI_DEVICE_ERROR      The device had an error.
> > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> or
> > the medium is no longer supported.
> >  **/
> >  EFI_STATUS
> >  EFIAPI
> > @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
> >    // Open the root volume now...
> >    //
> >    Status = SimpleFileSystem->OpenVolume(SimpleFileSystem,
> &RealFileHandle);
> > +  if (EFI_ERROR(Status)) {
> > +    return Status;
> > +  }
> > +
> >    *FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> > EfiShellGetMapFromDevicePath(&DevPath));
> > -  return (Status);
> > +  return (EFI_SUCCESS);
> >  }
> >
> >  /**
> > --
> > 2.12.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
Posted by Ni, Ruiyu 7 years, 1 month ago 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: Wu, Hao A
> Sent: Tuesday, November 14, 2017 4:41 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Carsey, Jaben <jaben.carsey@intel.com>
> Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> OpenRootByHandle()
> 
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Tuesday, November 14, 2017 4:38 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben
> > Subject: RE: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > OpenRootByHandle()
> >
> > How about changing the function header comments from
> > > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> > or the medium is no longer supported.
> >
> > To:
> > > +  @retval others  Error status returned from
> > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL->OpenVolume().
> >
> > Because MEDIA_CHANGED is not the only error that could happen.
> 
> Yes, I will update the comments when I push the patch if there's no other
> comments.
> 
> 
> Best Regards,
> Hao Wu
> 
> >
> >
> > Thanks/Ray
> >
> > > -----Original Message-----
> > > From: Wu, Hao A
> > > Sent: Tuesday, November 14, 2017 3:54 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Carsey, Jaben
> > > <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > > Subject: [PATCH] ShellPkg/Shell: Check the OpenVolume result in
> > > OpenRootByHandle()
> > >
> > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=779
> > >
> > > For the API EfiShellOpenRootByHandle():
> > >
> > > The return status of the call to SimpleFileSystem->OpenVolume should
> > > be checked.
> > >
> > > It is possible that there is a media change in the device (like CD/DVD ROM).
> > In
> > > such case, the volume root opened and/or the device path opened
> > > previously (also within EfiShellOpenRootByHandle) may be invalid.
> > >
> > > This commit adds a check for the result of OpenVolume before
> > > subsequently calling functions like EfiShellGetMapFromDevicePath() &
> > > ConvertEfiFileProtocolToShellHandle().
> > >
> > > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > > ---
> > >  ShellPkg/Application/Shell/ShellProtocol.c | 9 +++++++--
> > >  1 file changed, 7 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/ShellPkg/Application/Shell/ShellProtocol.c
> > > b/ShellPkg/Application/Shell/ShellProtocol.c
> > > index 5e34b8dad1..0dee267353 100644
> > > --- a/ShellPkg/Application/Shell/ShellProtocol.c
> > > +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> > > @@ -827,7 +827,8 @@ EfiShellGetDeviceName(
> > >    @retval EFI_NOT_FOUND         EFI_SIMPLE_FILE_SYSTEM could not be
> > found
> > > or the root directory
> > >                                  could not be opened.
> > >    @retval EFI_VOLUME_CORRUPTED  The data structures in the volume
> > were
> > > corrupted.
> > > -  @retval EFI_DEVICE_ERROR      The device had an error
> > > +  @retval EFI_DEVICE_ERROR      The device had an error.
> > > +  @retval EFI_MEDIA_CHANGED     The device has a different medium in it
> > or
> > > the medium is no longer supported.
> > >  **/
> > >  EFI_STATUS
> > >  EFIAPI
> > > @@ -867,8 +868,12 @@ EfiShellOpenRootByHandle(
> > >    // Open the root volume now...
> > >    //
> > >    Status = SimpleFileSystem->OpenVolume(SimpleFileSystem,
> > &RealFileHandle);
> > > +  if (EFI_ERROR(Status)) {
> > > +    return Status;
> > > +  }
> > > +
> > >    *FileHandle = ConvertEfiFileProtocolToShellHandle(RealFileHandle,
> > > EfiShellGetMapFromDevicePath(&DevPath));
> > > -  return (Status);
> > > +  return (EFI_SUCCESS);
> > >  }
> > >
> > >  /**
> > > --
> > > 2.12.0.windows.1

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

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.