* [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
@ 2017-11-14 7:53 Hao Wu
2017-11-14 8:37 ` Ni, Ruiyu
0 siblings, 1 reply; 4+ messages in thread
From: Hao Wu @ 2017-11-14 7:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Hao Wu, Jaben Carsey, Ruiyu Ni
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
2017-11-14 7:53 [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle() Hao Wu
@ 2017-11-14 8:37 ` Ni, Ruiyu
2017-11-14 8:41 ` Wu, Hao A
0 siblings, 1 reply; 4+ messages in thread
From: Ni, Ruiyu @ 2017-11-14 8:37 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org; +Cc: Carsey, Jaben
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
2017-11-14 8:37 ` Ni, Ruiyu
@ 2017-11-14 8:41 ` Wu, Hao A
2017-11-14 8:42 ` Ni, Ruiyu
0 siblings, 1 reply; 4+ messages in thread
From: Wu, Hao A @ 2017-11-14 8:41 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Carsey, Jaben
> -----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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle()
2017-11-14 8:41 ` Wu, Hao A
@ 2017-11-14 8:42 ` Ni, Ruiyu
0 siblings, 0 replies; 4+ messages in thread
From: Ni, Ruiyu @ 2017-11-14 8:42 UTC (permalink / raw)
To: Wu, Hao A, edk2-devel@lists.01.org; +Cc: Carsey, Jaben
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-11-14 10:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-14 7:53 [PATCH] ShellPkg/Shell: Check the OpenVolume result in OpenRootByHandle() Hao Wu
2017-11-14 8:37 ` Ni, Ruiyu
2017-11-14 8:41 ` Wu, Hao A
2017-11-14 8:42 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox