* [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