From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"afish@apple.com" <afish@apple.com>,
"Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] Is CoreValidateHandle() safe?
Date: Tue, 12 Jan 2021 01:51:33 +0000 [thread overview]
Message-ID: <BL0PR11MB3236C2E9BB6597E3628925EFD2AA0@BL0PR11MB3236.namprd11.prod.outlook.com> (raw)
In-Reply-To: <3BAF6AE3-F864-453D-8442-779442E9E342@apple.com>
[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]
Hi Andrew,
Isn’t the more typical condition for running into this CR ASSERT is that the calling code cached a copy of a handle that the calling code had freed before the call was made?
I agree it look like there may be a tiny window for a timer event. But even if we move the lock before CoreValidateHandle(), the timer could be signaled
right before the call was made. Once again, seems like the design of the calling code and its events need to make sure a freed handle is never passed in.
Mike
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrew Fish via groups.io
Sent: Monday, January 11, 2021 4:04 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>
Subject: [edk2-devel] Is CoreValidateHandle() safe?
I just hit the CR ASSERT [1] in CoreValidateHandle(). It looks like the IHANDLE was a use after free as it was a Pool buffer that was to small to be an IHANDLE and it did not have a valid handle.
I’m trying to understand why it is safe to walk the gHandleList without a lock? Seems like a local could cache a pointer and an event could remove a handle and Link would point to a stale handle?
Kind of feels like I’m missing something?
[1] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Hand/Handle.c#L64
EFI_STATUS
CoreValidateHandle (
IN EFI_HANDLE UserHandle
)
{
IHANDLE *Handle;
LIST_ENTRY *Link;
if (UserHandle == NULL) {
return EFI_INVALID_PARAMETER;
}
for (Link = gHandleList.BackLink; Link != &gHandleList; Link = Link->BackLink) {
Handle = CR (Link, IHANDLE, AllHandles, EFI_HANDLE_SIGNATURE);
if (Handle == (IHANDLE *) UserHandle) {
return EFI_SUCCESS;
}
}
return EFI_INVALID_PARAMETER;
}
Thanks,
Andrew Fish
[-- Attachment #2: Type: text/html, Size: 60146 bytes --]
next prev parent reply other threads:[~2021-01-12 1:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-12 0:03 Is CoreValidateHandle() safe? Andrew Fish
2021-01-12 1:51 ` Michael D Kinney [this message]
2021-01-12 7:08 ` [edk2-devel] " Andrew Fish
[not found] ` <16596A642FC06686.22089@groups.io>
2021-01-14 3:56 ` Andrew Fish
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=BL0PR11MB3236C2E9BB6597E3628925EFD2AA0@BL0PR11MB3236.namprd11.prod.outlook.com \
--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