public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Heyi Guo <heyi.guo@linaro.org>
To: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: Daryl McDaniel <edk2-lists@mc2research.org>,
	Jaben Carsey <jaben.carsey@intel.com>
Subject: [RFC] Add EFI lock when creating new gauge record
Date: Fri, 24 Nov 2017 15:04:18 +0800	[thread overview]
Message-ID: <6c7964be-bc2b-3ee4-e09e-c309fec7210f@linaro.org> (raw)

Hi folks,

We got occasional system exceptions after enabling performance measuring 
feature in edk2. After debugging, we found there is potential memory 
overflow in DXE/DXE_CORE PerformanceLib when PERF_START is reentered, 
and reentrance is possible 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. 
However I can't reproduce the issue right now; please let me know if 
PERF reentrance is not theoretically possible in the latest edk2 code.

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.

My proposal is to add EFI lock with TPL notify in StartGaugeEx to avoid 
such situation. The test result seemed good on our platforms and the 
performance measuring data was not impacted much by this patch.

Please let me know your comments.

Thanks,

Gary (Heyi Guo)



             reply	other threads:[~2017-11-24  7:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-24  7:04 Heyi Guo [this message]
2017-11-24  9:41 ` [RFC] Add EFI lock when creating new gauge record Zeng, Star
2017-11-25  9:09   ` Heyi Guo
2017-11-27  2:06     ` Zeng, Star
2017-11-27  3:14       ` Heyi Guo
2017-11-27  6:58     ` Gao, Liming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6c7964be-bc2b-3ee4-e09e-c309fec7210f@linaro.org \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox