From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web12.2341.1606338040720286831 for ; Wed, 25 Nov 2020 13:00:41 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XcqoiCGK; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1606338039; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5Qhpiadm55qkRj/rFuQhCOm8pN3R2yUN5KVPRVc+kec=; b=XcqoiCGKOybPP10CQWQ5b1jLlxS7Jjl0qf3cgQrF8WRlU+BZhBZoplcLF5+pA+fJM2bFXX YbAfmP1F45yoh+N8JIXNbvkomWoYHrt6QS/Ym1Fiie62hce4+cbTHyDGAutSRodiGr7JXD xkehzKmqSo/PpWaPB+J5fKMQUS8eRCA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-594-8augzahjPtW2UHkFi-nwHg-1; Wed, 25 Nov 2020 16:00:34 -0500 X-MC-Unique: 8augzahjPtW2UHkFi-nwHg-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 203901E7DE; Wed, 25 Nov 2020 21:00:33 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-239.ams2.redhat.com [10.36.112.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2A15019D9C; Wed, 25 Nov 2020 21:00:30 +0000 (UTC) Subject: Re: [PATCH] MdeModulePkg: Fix runtime panic in ValidateSetVariable() To: jejb@linux.ibm.com, "Liming Gao (Byosoft address)" Cc: devel@edk2.groups.io, Bret Barkelew , "Ard Biesheuvel (ARM address)" , Hao A Wu , Jian J Wang References: <414b7574bf8249de0cecd16fb422c711feb76e1a.camel@linux.ibm.com> From: "Laszlo Ersek" Message-ID: <3090ce8d-74de-0fc7-985e-c4831091e478@redhat.com> Date: Wed, 25 Nov 2020 22:00:30 +0100 MIME-Version: 1.0 In-Reply-To: <414b7574bf8249de0cecd16fb422c711feb76e1a.camel@linux.ibm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/25/20 21:13, James Bottomley wrote: > The current variable policy is allocated by AllocatePool(), which is > boot time only. This means that if you do any variable setting in the > runtime, the policy has been freed. Ordinarily this isn't detected > because freed memory is still there, but when you boot the Linux > kernel, it's been remapped so the actual memory no longer exists in > the memory map causing a page fault. > > Fix this by making it AllocateRuntimePool(). For SMM drivers, the > platform DSC is responsible for resolving the MemoryAllocationLib > class to the SmmMemoryAllocationLib instance. In the > SmmMemoryAllocationLib instance, AllocatePool() and > AllocateRuntimePool() are implemented identically. Therefore this > change is a no-op when the RegisterVariablePolicy() function is built > into an SMM driver. The fix affects runtime DXE drivers only. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3092 > Signed-off-by: James Bottomley > --- > MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c > index 5029ddb96adb..12944ac7ea81 100644 > --- a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c > +++ b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c > @@ -411,7 +411,7 @@ RegisterVariablePolicy ( > } > > // Reallocate and copy the table. > - NewTable = AllocatePool( NewSize ); > + NewTable = AllocateRuntimePool( NewSize ); > if (NewTable == NULL) { > return EFI_OUT_OF_RESOURCES; > } > (1) CC'ing Jian and Hao: $ python BaseTools/Scripts/GetMaintainer.py \ -l MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c Jian J Wang Hao A Wu Liming Gao devel@edk2.groups.io (2) My feedback: Fixes: 355b181f74050cdf2f09b1755c1a5ee4affb1faf Reviewed-by: Laszlo Ersek Tested-by: Laszlo Ersek (I tested the actual bugfix with SMM-less OVMF. I also regression-tested the patch, namely with SMM OVMF, and ArmVirtQemu too.) (3) I suggest updating the subject line as follows: MdeModulePkg/VariablePolicyLib: Fix runtime panic in ValidateSetVariable() 74 characters, so it's not overlong. No need to repost because of this. Liming, can you please pick up my feedback tags from (2), in addition to your own review, and refresh the subject as requested in (3), and then merge this patch -- before releasing edk2-stable202011? Thank you all, Laszlo