From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.88, mailfrom: jaben.carsey@intel.com) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by groups.io with SMTP; Thu, 08 Aug 2019 07:50:23 -0700 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Aug 2019 07:50:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,361,1559545200"; d="scan'208";a="169009934" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by orsmga008.jf.intel.com with ESMTP; 08 Aug 2019 07:50:21 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 8 Aug 2019 07:50:09 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.141]) by FMSMSX110.amr.corp.intel.com ([169.254.14.63]) with mapi id 14.03.0439.000; Thu, 8 Aug 2019 07:50:08 -0700 From: "Carsey, Jaben" To: "devel@edk2.groups.io" , "Gao, Zhichao" CC: "Ni, Ray" Subject: Re: [edk2-devel] [PATCH] ShellPkg/UefiShellLevel2CommansLib: Pointer Resonse should be checked Thread-Topic: [edk2-devel] [PATCH] ShellPkg/UefiShellLevel2CommansLib: Pointer Resonse should be checked Thread-Index: AQHVTYPR9sJJF3lDxUy/a1hNcUJaBabxVTcw Date: Thu, 8 Aug 2019 14:50:07 +0000 Message-ID: References: <20190808005405.27080-1-zhichao.gao@intel.com> In-Reply-To: <20190808005405.27080-1-zhichao.gao@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzQ3MWQ1MmYtNWNmOS00MzhlLWE0ZTAtYzhiYTYzYjUyNzgwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNGZ0N0tHSEM5TFRrK0lcL2RyZnhyUHdrVXBXTjFaM0hCeGVyZHlXSWE1VTc5QWhkME5weGRpb1lIVjhxclwvR0xPIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [10.1.200.108] MIME-Version: 1.0 Return-Path: jaben.carsey@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable That looks good, but I think it would be easier to just do the logic opposi= te. Cant we just add the ~"if it is NULL, then return" and not have to cha= nge anything else? No real need for else in that case since there is a retu= rn statement... Hand written pseudo-patch follows: if (!SilentMode) { + if (Response =3D=3D NULL) { + return SHELL_ABORTED; + } switch (*(SHELL_PROMPT_RESPONSE*)Response) { Thanks -Jaben > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gao, Zhichao > Sent: Wednesday, August 07, 2019 5:54 PM > To: devel@edk2.groups.io > Cc: Carsey, Jaben ; Ni, Ray > Subject: [edk2-devel] [PATCH] ShellPkg/UefiShellLevel2CommansLib: Pointe= r > Resonse should be checked >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2049 >=20 > ShellPkg\Library\UefiShellLevel2CommandsLib\Cp.c line 104 and > ShellPkg\Library\UefiShellLevel2CommandsLib\Mv.c line 640, the > pointer variable Response may be a NULL pointer. So we should > make sure that it isn't NULL before dereference it. >=20 > If Response is NULL that indicates a EFI_OUT_OF_RESOURCES > error, directly return SHELL_ABORTED. >=20 > Cc: Jaben Carsey > Cc: Ray Ni > Signed-off-by: Zhichao Gao > --- > .../Library/UefiShellLevel2CommandsLib/Cp.c | 42 ++++++++------- > .../Library/UefiShellLevel2CommandsLib/Mv.c | 52 ++++++++++--------- > 2 files changed, 51 insertions(+), 43 deletions(-) >=20 > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > index 18b05b5803..632d50229a 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Cp.c > @@ -2,7 +2,7 @@ > Main file for cp shell level 2 function. >=20 > (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved. > + Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -101,24 +101,28 @@ CopySingleFile( > // possibly return based on response > // > if (!SilentMode) { > - switch (*(SHELL_PROMPT_RESPONSE*)Response) { > - case ShellPromptResponseNo: > - // > - // return success here so we dont stop the process > - // > - return (SHELL_SUCCESS); > - case ShellPromptResponseCancel: > - *Resp =3D Response; > - // > - // indicate to stop everything > - // > - return (SHELL_ABORTED); > - case ShellPromptResponseAll: > - *Resp =3D Response; > - case ShellPromptResponseYes: > - break; > - default: > - return SHELL_ABORTED; > + if (Response !=3D NULL) { > + switch (*(SHELL_PROMPT_RESPONSE*)Response) { > + case ShellPromptResponseNo: > + // > + // return success here so we dont stop the process > + // > + return (SHELL_SUCCESS); > + case ShellPromptResponseCancel: > + *Resp =3D Response; > + // > + // indicate to stop everything > + // > + return (SHELL_ABORTED); > + case ShellPromptResponseAll: > + *Resp =3D Response; > + case ShellPromptResponseYes: > + break; > + default: > + return SHELL_ABORTED; > + } > + } else { > + return SHELL_ABORTED; > } > } > } > diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > index 8c2852d7eb..2cfa588a8c 100644 > --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Mv.c > @@ -2,7 +2,7 @@ > Main file for mv shell level 2 function. >=20 > (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
> - Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved. > + Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > **/ > @@ -637,29 +637,33 @@ ValidateAndMoveFiles( > if (Response =3D=3D NULL) { > ShellPromptForResponseHii(ShellPromptResponseTypeYesNoAllCancel= , > STRING_TOKEN (STR_GEN_DEST_EXIST_OVR), gShellLevel2HiiHandle, > &Response); > } > - switch (*(SHELL_PROMPT_RESPONSE*)Response) { > - case ShellPromptResponseNo: > - FreePool(Response); > - Response =3D NULL; > - continue; > - case ShellPromptResponseCancel: > - *Resp =3D Response; > - // > - // indicate to stop everything > - // > - SHELL_FREE_NON_NULL(FullCwd); > - return (SHELL_ABORTED); > - case ShellPromptResponseAll: > - *Resp =3D Response; > - break; > - case ShellPromptResponseYes: > - FreePool(Response); > - Response =3D NULL; > - break; > - default: > - FreePool(Response); > - SHELL_FREE_NON_NULL(FullCwd); > - return SHELL_ABORTED; > + if (Response !=3D NULL) { > + switch (*(SHELL_PROMPT_RESPONSE*)Response) { > + case ShellPromptResponseNo: > + FreePool(Response); > + Response =3D NULL; > + continue; > + case ShellPromptResponseCancel: > + *Resp =3D Response; > + // > + // indicate to stop everything > + // > + SHELL_FREE_NON_NULL(FullCwd); > + return (SHELL_ABORTED); > + case ShellPromptResponseAll: > + *Resp =3D Response; > + break; > + case ShellPromptResponseYes: > + FreePool(Response); > + Response =3D NULL; > + break; > + default: > + FreePool(Response); > + SHELL_FREE_NON_NULL(FullCwd); > + return SHELL_ABORTED; > + } > + } else { > + return SHELL_ABORTED; > } > Status =3D ShellDeleteFileByName(FullDestPath!=3DNULL? > FullDestPath:DestPath); > } > -- > 2.21.0.windows.1 >=20 >=20 >=20