From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0710.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe48::710]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 377DF81F6E for ; Thu, 8 Dec 2016 09:03:57 -0800 (PST) Received: from CS1PR84MB0229.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.151) by CS1PR84MB0232.NAMPRD84.PROD.OUTLOOK.COM (10.162.190.154) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.761.9; Thu, 8 Dec 2016 17:03:56 +0000 Received: from CS1PR84MB0229.NAMPRD84.PROD.OUTLOOK.COM ([10.162.190.151]) by CS1PR84MB0229.NAMPRD84.PROD.OUTLOOK.COM ([10.162.190.151]) with mapi id 15.01.0761.018; Thu, 8 Dec 2016 17:03:56 +0000 From: "Shah, Tapan" To: Ruiyu Ni , "edk2-devel@lists.01.org" CC: Jaben Carsey , Chen A Chen Thread-Topic: [edk2] [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable Thread-Index: AQHSUPTB6+ZBNU0Fo0eQo0AUFrRcPaD+SBlA Date: Thu, 8 Dec 2016 17:03:55 +0000 Message-ID: References: <20161208014519.281120-1-ruiyu.ni@intel.com> In-Reply-To: <20161208014519.281120-1-ruiyu.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=tapandshah@hpe.com; x-originating-ip: [15.203.227.10] x-ms-office365-filtering-correlation-id: 46a29ec7-d362-466d-79d9-08d41f8c31f4 x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:CS1PR84MB0232; x-microsoft-exchange-diagnostics: 1; CS1PR84MB0232; 7:j5grVznsgSQXVV1VDkO6qU5YaMraXc0+B6JheJAHWhXhUsDH8ty7CDEAdzz7EyhbN5YQJxHpjyMsmEHrRsFqGzCTpNkCFvslt24UQ07iu6vVY0e8OXAk6vRvvYxoWmiqsGcP11KREkUz6eZ8l68t6XTEYzt22i5FQZtIPXXgvqXAOdH9IAxTZnZaZBqGDqMkGuolUvoMggbsrWFjd2ennN43tK9+8zhwyxstN5JQE4RHnA57AlA5lRvjVCvJd8GymIvT6pusmtmEEzekZQQd0soojJHboqNFZSFNBDofm3A3h46oZaN5VpH54DQkcatzhxwFtKzDEo3FVlI+WUB1wHPSfuR17rflyooerNy+hStdPETISqJwO8yrh7BvfjnEXaTPFvkeGDEFT+jp397CTvKo/GFkHSgcmVE4h0ElDyfvIUEHeUJc7ieeEovLkH5bf+Me+lnccj0eLxsKx8faVA== x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(227479698468861)(162533806227266)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026)(6041248)(20161123560025)(20161123564025)(20161123562025)(20161123555025)(6072148); SRVR:CS1PR84MB0232; BCL:0; PCL:0; RULEID:; SRVR:CS1PR84MB0232; x-forefront-prvs: 0150F3F97D x-forefront-antispam-report: SFV:NSPM; SFS:(10019020)(6009001)(7916002)(39410400002)(39840400002)(39850400002)(39450400003)(39860400002)(377454003)(189002)(13464003)(199003)(3846002)(105586002)(102836003)(15650500001)(6116002)(68736007)(7846002)(7736002)(97736004)(66066001)(8936002)(77096006)(189998001)(92566002)(2950100002)(38730400001)(229853002)(106356001)(6506006)(86362001)(2900100001)(305945005)(3660700001)(106116001)(3280700002)(7696004)(2906002)(33656002)(122556002)(5001770100001)(4326007)(74316002)(5660300001)(81166006)(9686002)(50986999)(81156014)(2501003)(99286002)(101416001)(76176999)(54356999)(8676002)(21314002); DIR:OUT; SFP:1102; SCL:1; SRVR:CS1PR84MB0232; H:CS1PR84MB0229.NAMPRD84.PROD.OUTLOOK.COM; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: hpe.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: hpe.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Dec 2016 17:03:55.8188 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 105b2061-b669-4b31-92ac-24d304d195dc X-MS-Exchange-Transport-CrossTenantHeadersStamped: CS1PR84MB0232 Subject: Re: [PATCH] ShellPkg/Application: Fix ">v" cannot update environment variable X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Dec 2016 17:03:57 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Tapan Shah -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiy= u Ni Sent: Wednesday, December 07, 2016 7:45 PM To: edk2-devel@lists.01.org Cc: Jaben Carsey ; Chen A Chen Subject: [edk2] [PATCH] ShellPkg/Application: Fix ">v" cannot update enviro= nment variable From: Chen A Chen When ">v" is used to redirect the command output to environment variable (e.g.: "echo xxx >v yyy"), we only called SetVariable() to update the variable storage but forgot to update the cached environment variables in gShellEnvVarList. When updating the variable storage, the existing code unnecessary saved the ending NULL character into variable storage. The patch fixes all the above issues. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chen A Chen Cc: Ruiyu Ni Cc: Jaben Carsey --- ShellPkg/Application/Shell/FileHandleWrappers.c | 170 ++++++++++++++++++--= ---- ShellPkg/Application/Shell/ShellEnvVar.c | 12 +- ShellPkg/Application/Shell/ShellProtocol.c | 4 +- 3 files changed, 142 insertions(+), 44 deletions(-) diff --git a/ShellPkg/Application/Shell/FileHandleWrappers.c b/ShellPkg/App= lication/Shell/FileHandleWrappers.c index 18f4169..3c11d82 100644 --- a/ShellPkg/Application/Shell/FileHandleWrappers.c +++ b/ShellPkg/Application/Shell/FileHandleWrappers.c @@ -1040,6 +1040,7 @@ FileInterfaceEnvClose( UINTN NewSize; EFI_STATUS Status; BOOLEAN Volatile; + UINTN TotalSize; =20 // // Most if not all UEFI commands will have an '\r\n' at the end of any o= utput.=20 @@ -1049,6 +1050,7 @@ FileInterfaceEnvClose( // NewBuffer =3D NULL; NewSize =3D 0; + TotalSize =3D 0; =20 Status =3D IsVolatileEnv (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, = &Volatile); if (EFI_ERROR (Status)) { @@ -1057,7 +1059,8 @@ FileInterfaceEnvClose( =20 Status =3D SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMEN= T*)This)->Name, &NewSize, NewBuffer); if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { - NewBuffer =3D AllocateZeroPool(NewSize + sizeof(CHAR16)); + TotalSize =3D NewSize + sizeof (CHAR16); + NewBuffer =3D AllocateZeroPool (TotalSize); if (NewBuffer =3D=3D NULL) { return EFI_OUT_OF_RESOURCES; } =20 @@ -1066,17 +1069,43 @@ FileInterfaceEnvClose( =20 if (!EFI_ERROR(Status) && NewBuffer !=3D NULL) { =20 - if (StrSize(NewBuffer) > 6) - { - if ((((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 2] =3D=3D CHAR_LI= NEFEED)=20 - && (((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] =3D=3D CHA= R_CARRIAGE_RETURN)) { - ((CHAR16*)NewBuffer)[(StrSize(NewBuffer)/2) - 3] =3D CHAR_NULL; = =20 + if (TotalSize / sizeof (CHAR16) >=3D 3) { + if ( (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 2] =3D=3D C= HAR_LINEFEED) && + (((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] =3D=3D C= HAR_CARRIAGE_RETURN) + ) { + ((CHAR16*)NewBuffer)[TotalSize / sizeof (CHAR16) - 3] =3D CHAR_NUL= L; } =20 if (Volatile) { - Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_EN= VIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer); + Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_V ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + TotalSize - sizeof (CHAR16), + NewBuffer + ); + + if (!EFI_ERROR(Status)) { + Status =3D ShellAddEnvVarToList ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + NewBuffer, + TotalSize, + EFI_VARIABLE_BOOTSERVICE_ACCESS + ); + } } else { - Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_NV(((EFI_FILE_PROTOCOL_E= NVIRONMENT*)This)->Name, StrSize(NewBuffer), NewBuffer); + Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_NV ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + TotalSize - sizeof (CHAR16), + NewBuffer + ); + + if (!EFI_ERROR(Status)) { + Status =3D ShellAddEnvVarToList ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + NewBuffer, + TotalSize, + EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_= ACCESS + ); + } } } }=20 @@ -1128,12 +1157,14 @@ FileInterfaceEnvRead( =20 /** File style interface for Volatile Environment Variable (Write). + This function also caches the environment variable into gShellEnvVarList= . =20 @param[in] This The pointer to the EFI_FILE_PROTOCOL object= . @param[in, out] BufferSize Size in bytes of Buffer. @param[in] Buffer The pointer to the buffer to write. =20 - @retval EFI_SUCCESS The data was read. + @retval EFI_SUCCESS The data was successfully write to varia= ble. + @retval SHELL_OUT_OF_RESOURCES A memory allocation failed. **/ EFI_STATUS EFIAPI @@ -1146,41 +1177,71 @@ FileInterfaceEnvVolWrite( VOID* NewBuffer; UINTN NewSize; EFI_STATUS Status; + UINTN TotalSize; =20 NewBuffer =3D NULL; NewSize =3D 0; + TotalSize =3D 0; =20 Status =3D SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMEN= T*)This)->Name, &NewSize, NewBuffer); - if (Status =3D=3D EFI_BUFFER_TOO_SMALL){ - NewBuffer =3D AllocateZeroPool(NewSize + *BufferSize + sizeof(CHAR16))= ; + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { + TotalSize =3D NewSize + *BufferSize + sizeof (CHAR16); + } else if (Status =3D=3D EFI_NOT_FOUND) { + TotalSize =3D *BufferSize + sizeof(CHAR16); + } else { + return Status; + } + + NewBuffer =3D AllocateZeroPool (TotalSize); + if (NewBuffer =3D=3D NULL) { + return EFI_OUT_OF_RESOURCES; + } + + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { Status =3D SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONM= ENT*)This)->Name, &NewSize, NewBuffer); } - if (!EFI_ERROR(Status) && NewBuffer !=3D NULL) { - while (((CHAR16*)NewBuffer)[NewSize/2] =3D=3D CHAR_NULL) { - // - // We want to overwrite the CHAR_NULL - // - NewSize -=3D 2; - } - CopyMem((UINT8*)NewBuffer + NewSize + 2, Buffer, *BufferSize); - Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRO= NMENT*)This)->Name, StrSize(NewBuffer), NewBuffer); - FreePool(NewBuffer); - return (Status); - } else { - SHELL_FREE_NON_NULL(NewBuffer); - return (SHELL_SET_ENVIRONMENT_VARIABLE_V(((EFI_FILE_PROTOCOL_ENVIRONME= NT*)This)->Name, *BufferSize, Buffer)); + + if (EFI_ERROR (Status) && Status !=3D EFI_NOT_FOUND) { + FreePool (NewBuffer); + return Status; + } + + CopyMem ((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize); + Status =3D ShellAddEnvVarToList ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + NewBuffer, + TotalSize, + EFI_VARIABLE_BOOTSERVICE_ACCESS + ); + if (EFI_ERROR(Status)) { + FreePool (NewBuffer); + return Status; + } + + Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_V ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + TotalSize - sizeof (CHAR16), + NewBuffer + ); + if (EFI_ERROR(Status)) { + ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Nam= e); } + + FreePool (NewBuffer); + return Status; } =20 =20 /** File style interface for Non Volatile Environment Variable (Write). + This function also caches the environment variable into gShellEnvVarList= . =20 @param[in] This The pointer to the EFI_FILE_PROTOCOL object= . @param[in, out] BufferSize Size in bytes of Buffer. @param[in] Buffer The pointer to the buffer to write. =20 - @retval EFI_SUCCESS The data was read. + @retval EFI_SUCCESS The data was successfully write to varia= ble. + @retval SHELL_OUT_OF_RESOURCES A memory allocation failed. **/ EFI_STATUS EFIAPI @@ -1193,27 +1254,58 @@ FileInterfaceEnvNonVolWrite( VOID* NewBuffer; UINTN NewSize; EFI_STATUS Status; + UINTN TotalSize; =20 NewBuffer =3D NULL; NewSize =3D 0; + TotalSize =3D 0; =20 Status =3D SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONMEN= T*)This)->Name, &NewSize, NewBuffer); - if (Status =3D=3D EFI_BUFFER_TOO_SMALL){ - NewBuffer =3D AllocateZeroPool(NewSize + *BufferSize); + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { + TotalSize =3D NewSize + *BufferSize + sizeof (CHAR16); + } else if (Status =3D=3D EFI_NOT_FOUND) { + TotalSize =3D *BufferSize + sizeof (CHAR16); + } else { + return Status; + } + + NewBuffer =3D AllocateZeroPool (TotalSize); + if (NewBuffer =3D=3D NULL) { + return EFI_OUT_OF_RESOURCES; + } + + if (Status =3D=3D EFI_BUFFER_TOO_SMALL) { Status =3D SHELL_GET_ENVIRONMENT_VARIABLE(((EFI_FILE_PROTOCOL_ENVIRONM= ENT*)This)->Name, &NewSize, NewBuffer); } - if (!EFI_ERROR(Status)) { - CopyMem((UINT8*)NewBuffer + NewSize, Buffer, *BufferSize); - return (SHELL_SET_ENVIRONMENT_VARIABLE_NV( - ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, - NewSize + *BufferSize, - NewBuffer)); - } else { - return (SHELL_SET_ENVIRONMENT_VARIABLE_NV( - ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, - *BufferSize, - Buffer)); + + if (EFI_ERROR(Status) && Status !=3D EFI_NOT_FOUND) { + FreePool (NewBuffer); + return Status; + } + + CopyMem ((UINT8*) NewBuffer + NewSize, Buffer, *BufferSize); + Status =3D ShellAddEnvVarToList ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + NewBuffer, + TotalSize, + EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS + ); + if (EFI_ERROR (Status)) { + FreePool (NewBuffer); + return Status; } + + Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_NV ( + ((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Name, + TotalSize - sizeof (CHAR16), + NewBuffer + ); + if (EFI_ERROR (Status)) { + ShellRemvoeEnvVarFromList (((EFI_FILE_PROTOCOL_ENVIRONMENT*)This)->Nam= e); + } + + FreePool (NewBuffer); + return Status; } =20 /** diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c b/ShellPkg/Applicatio= n/Shell/ShellEnvVar.c index 6d8c79b..4c49c1c 100644 --- a/ShellPkg/Application/Shell/ShellEnvVar.c +++ b/ShellPkg/Application/Shell/ShellEnvVar.c @@ -178,7 +178,10 @@ GetEnvironmentVariableList( Status =3D EFI_OUT_OF_RESOURCES; } else { ValSize =3D ValBufferSize; - VarList->Val =3D AllocateZeroPool(ValSize); + // + // We need another CHAR16 to save '\0' in VarList->Val. + // + VarList->Val =3D AllocateZeroPool (ValSize + sizeof (CHAR16)); if (VarList->Val =3D=3D NULL) { SHELL_FREE_NON_NULL(VarList); Status =3D EFI_OUT_OF_RESOURCES; @@ -188,7 +191,10 @@ GetEnvironmentVariableList( if (Status =3D=3D EFI_BUFFER_TOO_SMALL){ ValBufferSize =3D ValSize > ValBufferSize * 2 ? ValSize : ValBuf= ferSize * 2; SHELL_FREE_NON_NULL (VarList->Val); - VarList->Val =3D AllocateZeroPool(ValBufferSize); + // + // We need another CHAR16 to save '\0' in VarList->Val. + // + VarList->Val =3D AllocateZeroPool (ValBufferSize + sizeof (CHAR1= 6)); if (VarList->Val =3D=3D NULL) { SHELL_FREE_NON_NULL(VarList); Status =3D EFI_OUT_OF_RESOURCES; @@ -272,7 +278,7 @@ SetEnvironmentVariableList( ; !IsNull(ListHead, &Node->Link) ; Node =3D (ENV_VAR_LIST*)GetNextNode(ListHead, &Node->Link) ){ - Size =3D StrSize(Node->Val); + Size =3D StrSize (Node->Val) - sizeof (CHAR16); if (Node->Atts & EFI_VARIABLE_NON_VOLATILE) { Status =3D SHELL_SET_ENVIRONMENT_VARIABLE_NV(Node->Key, Size, Node->= Val); } else { diff --git a/ShellPkg/Application/Shell/ShellProtocol.c b/ShellPkg/Applicat= ion/Shell/ShellProtocol.c index 04b66c5..12c7c40 100644 --- a/ShellPkg/Application/Shell/ShellProtocol.c +++ b/ShellPkg/Application/Shell/ShellProtocol.c @@ -2872,8 +2872,8 @@ InternalEfiShellSetEnv( ); if (!EFI_ERROR (Status)) { Status =3D Volatile - ? SHELL_SET_ENVIRONMENT_VARIABLE_V(Name, StrSize(Value), Valu= e) - : SHELL_SET_ENVIRONMENT_VARIABLE_NV(Name, StrSize(Value), Val= ue); + ? SHELL_SET_ENVIRONMENT_VARIABLE_V (Name, StrSize (Value) - s= izeof (CHAR16), Value) + : SHELL_SET_ENVIRONMENT_VARIABLE_NV (Name, StrSize (Value) - = sizeof (CHAR16), Value); if (EFI_ERROR (Status)) { ShellRemvoeEnvVarFromList(Name); } --=20 2.9.0.windows.1 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel