From patchwork Mon Nov 27 03:31:44 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: gary guo X-Patchwork-Id: 119652 Delivered-To: patch@linaro.org Received: by 10.140.22.164 with SMTP id 33csp1217892qgn; Sun, 26 Nov 2017 19:32:12 -0800 (PST) X-Google-Smtp-Source: AGs4zMbFLLnQxFJ4aJRA+zlH6mOnOuUiwHFoSA0VmoCU42IyLoaM7MQlQJMrz6zyS2eqrBJyODt0 X-Received: by 10.101.66.197 with SMTP id l5mr35430210pgp.240.1511753532054; Sun, 26 Nov 2017 19:32:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511753532; cv=none; d=google.com; s=arc-20160816; b=CHAxh8iy4yP4PZdnexbFozmfBMh6n6dkVYCMRI6Fgadvtnhvp1WOl+JWXYYZHEGOLv jkhfDYY7YVmyGaA/EAvg0t81MiF9rkQaA2QODXdWjcWA23AA3RV+B61k6BIWCywjxe91 Dryc6Z3f5BUpg7Oq4Ml7mHxJONNjx8c3LfgP9kmKm0y/Yw2GKXCfNG3179D9a8uy21jc HrACP8n+KZJvmUrs45JMPiEHG6j1HBJ+Aywb4gvvaTaLLDWi5nQ3Z/EXd17NIQpGrztZ rWE/LXCrCIrOABS4/609Bx1moLWYIv0nrX29Vxhp6fcwJxQ7cl7EJDH9o7UKgCv04Z1o RQyw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:message-id:date:to:from:dkim-signature :delivered-to:arc-authentication-results; bh=u9DfpeBoYT9CaKfg9jkvZWDHR1PWZcpouUxQU6a6TZs=; b=GxF69xlLtLdwCQahKim2JAB1GG1xxxvBmsv34/ZfsBaojiZZBomxCTGlF92Wpsp+Re eemYeKIHsU6BKKcTMYfo26VIsJ36z+V6yLqrWY8fXQcBHk4IfC1tRwOaWmMjXg0tATBm iqJK6EGfTf4vtpaa1JHr9wZDOzgFnrYqeF09XiOsCR+aUs+Gint16X+IMjTqGkh8v3Ao lCbomcDD+ML1WUMzeRdLCTtAr/ae4wxEn22fRK3IVYGMfixm6gOg/JGQia+2Q9YIz/yq Td9hWsiKoBhN/u/a+/WeZdhVZQK0ySmQFHmO7JsGZn3Mtq7m5K9RVCoz5y4Zmq7HnO1k 4S4g== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=EgSzkY7Z; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id w2si22683202plz.643.2017.11.26.19.32.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 26 Nov 2017 19:32:12 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=EgSzkY7Z; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id A18E521A1099D; Sun, 26 Nov 2017 19:27:49 -0800 (PST) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:400e:c05::22a; helo=mail-pg0-x22a.google.com; envelope-from=heyi.guo@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pg0-x22a.google.com (mail-pg0-x22a.google.com [IPv6:2607:f8b0:400e:c05::22a]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 16CA621A1099A for ; Sun, 26 Nov 2017 19:27:47 -0800 (PST) Received: by mail-pg0-x22a.google.com with SMTP id l19so18092959pgo.2 for ; Sun, 26 Nov 2017 19:32:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=hTpuq/qGtzLRmU15q2wRM4s6k6GjYgvToqROWxhq3pQ=; b=EgSzkY7ZDm1Y9zjp83dlNpwrjTuIFqEQAFbxAEUBL2vnqI7nbKTOy4538gmaQfSORk 4MeHrc3meBCv5O8115ptxmqkZlyobYppxEhlRmvhsNeK4T2eRtc1mNy91NxtN/6WFrft BqztO5h9nAOV1tueKYQET8SSOtQmdbohHlUzo= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=hTpuq/qGtzLRmU15q2wRM4s6k6GjYgvToqROWxhq3pQ=; b=chXXAwWsWcwZGnlcrYyPNG50c7bDuJsOU4D0JW5OOHboIX/cDOTxFOPSKYxZqldUp2 ljl3f+AntcKKv4PM15aix/XeZqBvFovXgPoV5asNx8eSykuWzo5YjXzYMES9X88PiLnj 2eyi1y1oR5msQfDBqwvVSWxpiWEEVSBK4+OewVta47y3poTNZkg1jD3dwpepS8NORETq w7zx2PyxyFtljPkQgQawMh6cFJyBIR4uffjCCvNH12GLWmy37P3WAEpQy0GtCAqsEicU CxjRyHBBp8RfKgxZ/iNrIXTS0uXwzrkYc/j94lMl1s/LKFQcW+Pof8m0fvc2dkdb3bpP fy4g== X-Gm-Message-State: AJaThX4cwrdrJr29LT0DoCjnWQWyy9SFNr9y+eotm3/n+lIKVr+Tka/8 yJT1b3P4u+M5c1zgvErJXnETTw== X-Received: by 10.99.127.84 with SMTP id p20mr35717706pgn.204.1511753528743; Sun, 26 Nov 2017 19:32:08 -0800 (PST) Received: from szxbz956.huaweiobz.com ([45.56.152.184]) by smtp.gmail.com with ESMTPSA id b8sm23294231pff.26.2017.11.26.19.32.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sun, 26 Nov 2017 19:32:08 -0800 (PST) From: Heyi Guo To: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org Date: Mon, 27 Nov 2017 11:31:44 +0800 Message-Id: <1511753504-38548-1-git-send-email-heyi.guo@linaro.org> X-Mailer: git-send-email 2.7.2.windows.1 Subject: [edk2] [PATCH] MdeModulePkg/PerformanceLib: add lock protection X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Heyi Guo , Liming Gao , Eric Dong , Star Zeng MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" DXE performance gauge record access functions might be reentered since we are supporting something like USB hot-plug, which is a timer event where gBS->ConnectController might be called and then PERF will be called in CoreConnectSingleController. When StartGaugeEx is being reentered, not only the gauge record might be overwritten, more serious situation will be caused if gauge data buffer reallocation procedure is interrupted, between line 180 and 187 in DxeCorePerformanceLib.c specifically. There, mMaxGaugeRecords will be doubled twice (denoted as 4X), but mGaugeData only points to a buffer of size 2X, which will probably cause the following 2X memory to be overflowed when gauge records are increased. So we add EFI lock with with TPL_NOTIFY in StartGaugeEx/EndGaugeEx/GetGaugeEx to avoid memory overflow and gauge data corruption. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Heyi Guo Cc: Star Zeng Cc: Eric Dong Cc: Liming Gao --- .../DxeCorePerformanceLib/DxeCorePerformanceLib.c | 67 +++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Star Zeng with the redundant with at " with with " in the commit log. Signed-off-by: Heyi Guo Signed-off-by: Heyi Guo diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c index 51f488a..7c0e207 100644 --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c @@ -63,6 +63,11 @@ PERFORMANCE_EX_PROTOCOL mPerformanceExInterface = { PERFORMANCE_PROPERTY mPerformanceProperty; +// +// Gauge record lock to avoid data corruption or even memory overflow +// +STATIC EFI_LOCK mPerfRecordLock = EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTIFY); + /** Searches in the gauge array with keyword Handle, Token, Module and Identifier. @@ -162,6 +167,12 @@ StartGaugeEx ( UINTN OldGaugeDataSize; GAUGE_DATA_HEADER *OldGaugeData; UINT32 Index; + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } Index = mGaugeData->NumberOfEntries; if (Index >= mMaxGaugeRecords) { @@ -175,6 +186,7 @@ StartGaugeEx ( NewGaugeData = AllocateZeroPool (GaugeDataSize); if (NewGaugeData == NULL) { + EfiReleaseLock (&mPerfRecordLock); return EFI_OUT_OF_RESOURCES; } @@ -209,6 +221,8 @@ StartGaugeEx ( mGaugeData->NumberOfEntries++; + EfiReleaseLock (&mPerfRecordLock); + return EFI_SUCCESS; } @@ -250,6 +264,12 @@ EndGaugeEx ( { GAUGE_DATA_ENTRY_EX *GaugeEntryExArray; UINT32 Index; + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } if (TimeStamp == 0) { TimeStamp = GetPerformanceCounter (); @@ -257,11 +277,13 @@ EndGaugeEx ( Index = InternalSearchForGaugeEntry (Handle, Token, Module, Identifier); if (Index >= mGaugeData->NumberOfEntries) { + EfiReleaseLock (&mPerfRecordLock); return EFI_NOT_FOUND; } GaugeEntryExArray = (GAUGE_DATA_ENTRY_EX *) (mGaugeData + 1); GaugeEntryExArray[Index].EndTimeStamp = TimeStamp; + EfiReleaseLock (&mPerfRecordLock); return EFI_SUCCESS; } @@ -274,6 +296,8 @@ EndGaugeEx ( If it stands for a valid entry, then EFI_SUCCESS is returned and GaugeDataEntryEx stores the pointer to that entry. + This internal function is added to avoid releasing lock before each return statement. + @param LogEntryKey The key for the previous performance measurement log entry. If 0, then the first performance measurement log entry is retrieved. @param GaugeDataEntryEx The indirect pointer to the extended gauge data entry specified by LogEntryKey @@ -287,7 +311,7 @@ EndGaugeEx ( **/ EFI_STATUS EFIAPI -GetGaugeEx ( +InternalGetGaugeEx ( IN UINTN LogEntryKey, OUT GAUGE_DATA_ENTRY_EX **GaugeDataEntryEx ) @@ -314,6 +338,47 @@ GetGaugeEx ( } /** + Retrieves a previously logged performance measurement. + It can also retrieve the log created by StartGauge and EndGauge of PERFORMANCE_PROTOCOL, + and then assign the Identifier with 0. + + Retrieves the performance log entry from the performance log specified by LogEntryKey. + If it stands for a valid entry, then EFI_SUCCESS is returned and + GaugeDataEntryEx stores the pointer to that entry. + + @param LogEntryKey The key for the previous performance measurement log entry. + If 0, then the first performance measurement log entry is retrieved. + @param GaugeDataEntryEx The indirect pointer to the extended gauge data entry specified by LogEntryKey + if the retrieval is successful. + + @retval EFI_SUCCESS The GuageDataEntryEx is successfully found based on LogEntryKey. + @retval EFI_NOT_FOUND The LogEntryKey is the last entry (equals to the total entry number). + @retval EFI_INVALIDE_PARAMETER The LogEntryKey is not a valid entry (greater than the total entry number). + @retval EFI_INVALIDE_PARAMETER GaugeDataEntryEx is NULL. + +**/ +EFI_STATUS +EFIAPI +GetGaugeEx ( + IN UINTN LogEntryKey, + OUT GAUGE_DATA_ENTRY_EX **GaugeDataEntryEx + ) +{ + EFI_STATUS Status; + + Status = EfiAcquireLockOrFail (&mPerfRecordLock); + if (EFI_ERROR (Status)) { + return Status; + } + + Status = InternalGetGaugeEx (LogEntryKey, GaugeDataEntryEx); + + EfiReleaseLock (&mPerfRecordLock); + + return Status; +} + +/** Adds a record at the end of the performance measurement log that records the start time of a performance measurement.