From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 1C277D80110 for ; Sat, 19 Apr 2025 03:32:13 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=0QF86DwlVw5sL3AfeZQH17NTuNSb/MXcko7seuC1EzQ=; c=relaxed/simple; d=groups.io; h=From:To:CC:Subject:Thread-Topic:Thread-Index:Date:Message-ID:References:In-Reply-To:Accept-Language:msip_labels:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type; s=20240830; t=1745033533; v=1; x=1745292732; b=XXF96kAwYFCV9KSCMYFyqXjJ6syAitY8LNz4i5unAu5290aD1ZJyjA8VfKj2AoX7dImXvbWI yKmVFfVimA7iRl0l4T7TS5+0kMa+nO3QhXvL3wbVxkbb+HCuHvXN6MW1VDrh3GmVAZRVyb3kq/p 8gHeksh1dH3sqqDtorkzoJA+FltO8AKohpW9n6peFb2ZHSVg9B1BZJQALFRRLhus9F+nOxZQzlK FTPGohDTzosDSBMMuHX8/yZlhJd/ThQEbQf0OKItj13FNXRTMJRfJRf79ui7LYjdBuT9qawZfB6 +XXwOhx+UiE+y1tV719x0I9YFJfmOsaiG/7823noDoJRA== X-Received: by 127.0.0.2 with SMTP id 5gkjYY7687511xbFof42yUVS; Fri, 18 Apr 2025 20:32:12 -0700 X-Received: from CY4PR02CU008.outbound.protection.outlook.com (CY4PR02CU008.outbound.protection.outlook.com [40.93.199.123]) by mx.groups.io with SMTP id smtpd.web11.777.1745014674805680183 for ; Fri, 18 Apr 2025 15:17:55 -0700 X-Received: from CH3PR21MB4517.namprd21.prod.outlook.com (2603:10b6:610:21b::17) by CH3PR21MB4018.namprd21.prod.outlook.com (2603:10b6:610:167::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8678.14; Fri, 18 Apr 2025 22:17:52 +0000 X-Received: from CH3PR21MB4517.namprd21.prod.outlook.com ([fe80::7bdd:bb65:8eb1:f082]) by CH3PR21MB4517.namprd21.prod.outlook.com ([fe80::7bdd:bb65:8eb1:f082%4]) with mapi id 15.20.8678.011; Fri, 18 Apr 2025 22:17:52 +0000 From: "Kun Qin via groups.io" To: "Kinney, Michael D" , "Ni, Ray" CC: "devel@edk2.groups.io" , "Ni, Ray" Subject: Re: [edk2-devel] A bug in the SmmCommunication V3 logic Thread-Topic: A bug in the SmmCommunication V3 logic Thread-Index: AQHbsCxo5nQBH4RMRk+1qEepe02xqLOpBL4QgACO8lCAAComQIAAMy/AgAAC4LA= Date: Fri, 18 Apr 2025 22:17:52 +0000 Message-ID: References: In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ActionId=7199950a-a9af-4e88-86e5-b41f15734340;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=true;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=Internal;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2025-04-18T07:21:47Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Tag=10, 3, 0, 1; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: CH3PR21MB4517:EE_|CH3PR21MB4018:EE_ x-ms-office365-filtering-correlation-id: 2165f259-3cb5-4bfd-c822-08dd7ec6dc5f x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?A3T5VPb3s6rPCOEcmsfUkcjyX54WyV6TPt+uw8lNxNfBZ/96WJxRfZUJuGdF?= =?us-ascii?Q?XB92Hiin7j18AZwHqzKKjhuQgPcNviLJclauTT7u6n05YVv8EPGsQnLV3xzf?= =?us-ascii?Q?sejx96KsFW6TeB61kFa0wQ1M1mB06ys6xRcr371+fiKMoXrQ6MryjRt5+ghh?= =?us-ascii?Q?OyWrIueMDTaHAQ3vUK585N246rlvyFaZRUw1/7B7RzgRriKqoJ6U3WH9o1ED?= =?us-ascii?Q?Dt35IgQlrXRHpgn5K7Tdbe3QEybvjYZcuYA1xvuhgaMgvYAZKXA90tSR0JDt?= =?us-ascii?Q?BcCrFwlS/B8iHZQ3hyamF/vcC2u/aS9b8/nHsGMeBY5NwuHOGW7oGqUy+jzm?= =?us-ascii?Q?riavoiOGwtPyJLU2+4gtedLosawayDQXrQwzUI0jvcK7cKvzDdyyoem8te1N?= =?us-ascii?Q?IGxiR3Nov9GMv8/q62OVXWO9q4Q+tauUKzJmWyeNb9VxOtWjR00MU2ovpO+4?= =?us-ascii?Q?5swgpVlP7MWOev/WKXuPypqDiLgrkUnvhl3WYAQNmouQO2SEUxbvHAF2OG69?= =?us-ascii?Q?JiUV2sCFr9kvF1Kf4m31ZKTEc8La5Gh37j5Ci6xwAusORg6GiGxTmyr3sTIY?= =?us-ascii?Q?xtiBPDCAkXXg3sC3CLX6ZhCMNISgxZEmeRh89+DF2aik3SlkJKv6Q/beC74I?= =?us-ascii?Q?tFvZjgstdJYOF4o7Ad/exWowt4em4CaU73TErpNAEQ/HEvCnJZ4xGprg12f+?= =?us-ascii?Q?IX9I9tLhGFzWACxoeIzC7Rkx6+cPasEJ+NhsnHr9Y4yYNeK363X7RS6n7Egq?= =?us-ascii?Q?dXb7W2vCiwsC9YPDQgt9iqerCayPkiC1egez0AGbc4+b6yljCXXdc+/XOUXW?= =?us-ascii?Q?qrY6FXaKx+cgdkjEjbluzZ9EQ8bOxdqTQom+ZhQliPh6rwU8kS3IG2TxoJ0E?= =?us-ascii?Q?Rh5yNhUPZwlhJ8Xap1aGqOVSOv5mu5IHssZO6MrRIVbSdzw6QqcwVh1x7xy7?= =?us-ascii?Q?mdjZbf4EQKR3u6lG3pOJ3k6m9hAiB7e7w2AaedkfI6LoJhruFadJSQa9QSqw?= =?us-ascii?Q?nnsMljJ3IFXM046+pxkolwO2/31o/Ik0VNo7q432Cpi/5sQcuDETI8YkeZBq?= =?us-ascii?Q?djpnXm4h8xLgqXVHtQppEZMz2EBAsT4fQppZtHqbRhZuWg1vP5PEBH2i9xrb?= =?us-ascii?Q?byBs2nxYX49xZZuPmnP9xzk9db6RtkgqWBGzw7xoVDAY6/Mw5nVqjn6tyUxq?= =?us-ascii?Q?aTMwNnkAOpXpgUtf+HXxUFi/FPYxeFR38KuAWJhkZXH9ksOsXSTfaHQDZjQM?= =?us-ascii?Q?KHfEw8tS15Fsf6iEZUVuHttfWyLbUwl7v8YdfoePV9yFUoynMX/NJ+Eco6SX?= =?us-ascii?Q?XUDeZnF0T3xK9bE3gUi8p3W13sisax9at0E3SRUgCh4dyopv60jlg1q53N0M?= =?us-ascii?Q?VvMBj0U11gBy0WH2h1Hr4gL2PE4aJ/RvWnNkOCcFRS5GPV0ixVVDjahdackk?= =?us-ascii?Q?sgOuZfuKbrJWTKspRWVEz3d9jqXXDN3Vs795k+exQIGmxQjqxgjdlA=3D=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?UHFFwcsDJcUEO2i9aZaR44Hug5veFzgYg+oGrkQzNKFMtayj+K6TqVuIIYCo?= =?us-ascii?Q?fYOrpNX7uMFyueYzeaGpHAO9hhARriZr+mYsyUu29H37LfZUFcWTFhzIFR2o?= =?us-ascii?Q?Tc5rm4aDLTQmZQUb4Kd6GAoO/4Wx5SPQTRJJ9Cs0UHx9Q+aupWW7jIRVuSB2?= =?us-ascii?Q?iYm2UsflbOFUk+BlMChna1AncIG+4dNiFNx/PmR/QZC1seHdL2D2dSgfBUFo?= =?us-ascii?Q?VU6VQsLwGFQHtfG6j8hvxER3dmLWvHGfljugWnv2feHTf4rR/06yhiTDv4i+?= =?us-ascii?Q?VnzwfDBkh+ZA/yglbFwXW+kxgqM85mZ5gpr+wcO2Yfr7wbaXu3y2EFFLiQD7?= =?us-ascii?Q?InbPd26LWaT7X+D/ikT0niz7ZnhNfjMHPPEToqAh53w1ErjBLfK4Wnho1BK2?= =?us-ascii?Q?ucAT0KdxcR9E3iniL3Vywh6WToT/KeE3N3dIZCQKI4VyaSQ5Md8ZwkG2hHKd?= =?us-ascii?Q?Vs0kh6HQeP3dzM8dz37uywzfZJMGNjU4FegcoeN+wswtAYe8dP8GCJFkZr3i?= =?us-ascii?Q?22f6XpF0O69JCJfiFVXn8i/ND3/+7t0fTchYWhgNan6t7Qh7G5YRkElEawXy?= =?us-ascii?Q?a+4ZOF35J9knVf8HWRfCprzSCeBzaWq5B1wUCDagJhD/9edEpUZN3x1Iouq4?= =?us-ascii?Q?cwZr3qBz21iP4/Wbjmx/dbIFuTCYC/vWxYP7nBkbSNrXdW5N3vwCbUfRYgRI?= =?us-ascii?Q?ApdPLBXY77Y7oRFneTB8H8JpR+VC1jiavA8MVf5wuckQwMJBlMHc4xCHdvcX?= =?us-ascii?Q?8JkSTm9MZSsuPOxS8S4qHG20P3SkQaY8t+mLGdJfOvGr+M0LsALmW4OTD477?= =?us-ascii?Q?jEGzdeJueN3ZKJ3uGgCmq0wK+PyKK8xqvfdk0QvBvNCPlHLcxOBm12tPBOJ8?= =?us-ascii?Q?akUmISfh/4zb/AyINbCaRymgeJr09IGpFhfgmSNdpvNo0B2LvX09rEzcyN7X?= =?us-ascii?Q?efEbK7XPfQz2M7F4yLsyvhmSSQB6eP7eGRsus9sBLSW9EAcOERAG/SsN8FRQ?= =?us-ascii?Q?XCBPWMLqJ8KpKBXSIpFdhD6f84OpJLTVo8cT2etFZO3BnGvQg+jnns8VOp2+?= =?us-ascii?Q?zzV5kAmgC4wk2y3/eydLD9nbxy8YYD9fQTKv5fID9z29BND9xpsOB76HiK9R?= =?us-ascii?Q?jeVjZViDF3aCu5HUoubNbOhGmtVGWuSDywzBpc0oSzpUlVHiCU+p5BjAV2qc?= =?us-ascii?Q?tWPxr6FKRU+Lc4EYcF0xKWXThh/R/ayjne/KP0x0DFpwV83pLtwaX0MFcnfG?= =?us-ascii?Q?mzVsh9WmaWbl9bqeNsAWf+1OMjjMwg2wDZr5LkkDFKfQH2kbD9PlH1rdwZhi?= =?us-ascii?Q?qYqWUu18nPD8tCn3btQbFmwXskIA8tA9FoReXsBox7Fvr4M2Q7h01saz5OB6?= =?us-ascii?Q?f8RxiH/AdYwUNH8gmMNsESow2jtQS8VUcw/cCiZOMt1wM3FnqKMzVZhhxOnr?= =?us-ascii?Q?hH0obKQy/Dypud9Cegdme7OC8M4sK+CgN3gn9cgA/vOYNqpH8iSVh5J2Hbpo?= =?us-ascii?Q?ZSDnLVp+zr6TDm0MeLtKcGgtSTEi4J347mHhIZSmodi7g0IfrBuUdGttENYP?= =?us-ascii?Q?a75bzzHQZ3gLLIQ1f84jt4njkndOsGrypJ+VhUEo4OKAl6ArXemXf2CHpFAj?= =?us-ascii?Q?DpkUzMAtYGFx6A1kT0N/ahk=3D?= MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CH3PR21MB4517.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2165f259-3cb5-4bfd-c822-08dd7ec6dc5f X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2025 22:17:52.1803 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: jIqLAgR2dNpYp6ISeJDRofMZtHG7YIoFHMb4A3CM6ypSrju5exJGsqT+mbUSVrF1jF8x7VhrzROTN4/qUICwbg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR21MB4018 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Fri, 18 Apr 2025 20:32:11 -0700 Resent-From: Kun.Qin@microsoft.com Reply-To: devel@edk2.groups.io,Kun.Qin@microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: AEQ2tzXNJihkhUTYhwmlV6pbx7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CH3PR21MB4517E0C71DCFC81317721932E9BF2CH3PR21MB4517namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=XXF96kAw; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=pass (policy=none) header.from=groups.io --_000_CH3PR21MB4517E0C71DCFC81317721932E9BF2CH3PR21MB4517namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Mike, I created a PR on edk2 site but I was thinking about redoing it... Because = of the v1 implementation is a bit chaotic. The original form are still broken if we back out the change, according to = our discussion below. Specifically for v1, we cannot touch the incoming buf= fer after going virtual at all, because we need the incoming buffer to be p= hysical to populate the gSmmCorePrivate->CommunicationBuffer pointer. Other= wise, the validation inside SMM will work pass. On the StMm side, we are partially covered because MM communicate on standa= lone mm will always intake the incoming and copy it over to the static buff= er `CopyMem ((VOID *)(UINTN)mMmCommonBuffer.PhysicalStart, CommBuffer, Buff= erSize);`. However, this bears the assumption that we will need to pass in = virtual address when invoking Stmm backed MM communicate v1 to make it work= , which is a contradictory expectation compared to traditional MM communica= te protocol. One solution is to make comm size a requirement for v1, and v2/3 can check = the incoming content through the virtual address. Please let me know your t= houghts. Thanks, Kun From: Kinney, Michael D Sent: Friday, April 18, 2025 2:34 PM To: Kun Qin ; Ni, Ray Cc: devel@edk2.groups.io; Ni, Ray ; Kinney, Michael D Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic Hi Kun, I have a proposed solution that puts SmmCommunicationCommunicate() back to = its original form, so there are no changes in behavior for SmmCommunication= Communicate() or SmmCommunicationMmCommunicate2(). I then pull the V3 only logic into MmCommunicationMmCommunicate3(). It has some code duplication, but makes it easier to understand and uses th= e virtual and physical pointers in the right places for the V3 implementati= on. What were you thinking? Mike From: Kun Qin > Sent: Friday, April 18, 2025 11:26 AM To: Kinney, Michael D >; Ni, Ray > Cc: devel@edk2.groups.io; Ni, Ray > Subject: RE: A bug in the SmmCommunication V3 logic Hi Mike, Thanks for the explanation. That makes sense. I confirmed the issue on PiSm= mIpl side. A patch should be available some time today after I test both tr= aditional MM and Standalone MM paths. Regards, Kun From: Kinney, Michael D > Sent: Friday, April 18, 2025 8:59 AM To: Kun Qin >; Ni, Ray = > Cc: devel@edk2.groups.io; Ni, Ray >; Kinney, Michael D > Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic Hi Kun, I think the reason it may have always worked is for the case where SmmCommu= nicationCommunicate() at runtime after SetVirtualAddressMap() and CommSize = is not NULL. In that case, CommunicateHeader that points to CommBuffer is = never dereferenced and a call to mSmmControl2->Trigger() is made. If CommSize is NULL, then MessageLength field of CommunicateHeader would be= dereferenced as a physical address at OS runtime after SetVirtualAddressMa= p() and a page fault would occur. My guess is that CommSize is never NULL,= and that is why we have been lucky. With the addition of the check for the V3 GUID, the HeaderGuid field of Com= municateHeader is always dereferenced. So if CommunicateHeader is a physic= al address at OS Runtime after SetVirtualAddressMap(), then a page fault wi= ll always occur. No more luck. Mike From: Kun Qin > Sent: Friday, April 18, 2025 12:30 AM To: Ni, Ray > Cc: Kinney, Michael D >; devel@edk2.groups.io; Ni, Ray > Subject: RE: A bug in the SmmCommunication V3 logic Hi Ray, I will verify the code first thing tomorrow morning. But I just looked at t= he code flow before the change, it seems that SmmCommunicationMmCommunicate= 2 is also using physical address, and the common routine will deference the= pointer to read message length as well. I just checked variable runtime dr= iver did not convert the input physical pointer. Would that cause the same = issue? Did I miss something or we have lucked out all along? Regards, Kun From: Ni, Ray > Sent: Thursday, April 17, 2025 11:45 PM To: Kun Qin > Cc: Kinney, Michael D >; devel@edk2.groups.io; Ni, Ray > Subject: [EXTERNAL] A bug in the SmmCommunication V3 logic Hi Qin, I think there is a bug in the SmmCommunication protocol implementation. All 3 communication protocol calls go to the same communicate() function th= at tests the HeaderGuid against the V3 GUID. But when the call is from runtime, reading the HeaderGuid using the physica= l address of communication buffer would cause page fault. The virtual addre= ss should be used. The bug was not there without your patch because the communicate routines h= appened not to read any bytes from the communication buffer but simply pass= the address to SMM. SMM expects the physical address because the virtual-t= o-physical mapping in SMM is identical. The bug exists in both the SmmIpl.c in MdeModulePkg and the MmCommunication= Dxe.c in StandaloneMmPkg. The bug would cause OS boot failure if there is any communication protocol = invocation after ExitBootService. I guess the bug might not be there in your first version of patch, but was = introduced when I asked you to consolidate the logic together. Can you kindly reproduce it locally and send out a fix after confirming? Thanks, Ray -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#121271): https://edk2.groups.io/g/devel/message/121271 Mute This Topic: https://groups.io/mt/112327494/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --_000_CH3PR21MB4517E0C71DCFC81317721932E9BF2CH3PR21MB4517namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

Hi Mike,

 

I created a PR on edk2 site but I was thinking abou= t redoing it… Because of the v1 implementation is a bit chaotic.=

 

The original form are still broken if we back out t= he change, according to our discussion below. Specifically for v1, we canno= t touch the incoming buffer after going virtual at all, because we need the incoming buffer to be physical to popul= ate the gSmmCorePrivate->CommunicationBuffer pointer. Otherwise, the val= idation inside SMM will work pass.

 

On the StMm side, we are partially covered because = MM communicate on standalone mm will always intake the incoming and copy it= over to the static buffer `CopyMem ((VOID *)(UINTN)mMmCommonBuffer.Physica= lStart, CommBuffer, BufferSize);`. However, this bears the assumption that we will= need to pass in virtual address when invoking Stmm backed MM communicate v1 to make = it work, which is a contradictory expectation compared to traditional MM co= mmunicate protocol.

 

One solution is to make comm size a requirement for= v1, and v2/3 can check the incoming content through the virtual address. P= lease let me know your thoughts.

 

Thanks,

Kun

 

From: Kinney, Michael D <michael.= d.kinney@intel.com>
Sent: Friday, April 18, 2025 2:34 PM
To: Kun Qin <Kun.Qin@microsoft.com>; Ni, Ray <ray.ni@intel.= com>
Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, = Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic<= /o:p>

 

Hi Kun,<= /span>

 

I have a proposed s= olution that puts SmmCommunicationCommunicate() back to its original form, = so there are no changes in behavior for SmmCommunicationCommunicate() or Sm= mCommunicationMmCommunicate2().

 

I then pull the V3 = only logic into MmCommunicationMmCommunicate3().

 

It has some code du= plication, but makes it easier to understand and uses the virtual and physi= cal pointers in the right places for the V3 implementation.

 

What were you think= ing?

 

Mike

 

 

From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, April 18, 2025 11:26 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray <ray.ni@intel.com>
Cc: devel@edk2.group= s.io; Ni, Ray <r= ay.ni@intel.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

Hi Mike,

 

Thanks for the explanation. That makes sense. I con= firmed the issue on PiSmmIpl side. A patch should be available some time to= day after I test both traditional MM and Standalone MM paths.

 

Regards,

Kun

 

From: Kinney, Michael D <<= a href=3D"mailto:michael.d.kinney@intel.com">michael.d.kinney@intel.com>
Sent: Friday, April 18, 2025 8:59 AM
To: Kun Qin <Kun= .Qin@microsoft.com>; Ni, Ray <ray.ni@intel.com>
Cc: devel@edk2.group= s.io; Ni, Ray <r= ay.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] RE: A bug in the SmmCommunication V3 logic<= /o:p>

 

Hi Kun,<= /span>

 

I think the reason = it may have always worked is for the case where SmmCommunicationCommunicate= () at runtime after SetVirtualAddressMap() and CommSize is not NULL.  = In that case, CommunicateHeader that points to CommBuffer is never dereferenced and a call to mSmmControl2->Trigger= () is made.

 

If CommSize is NULL= , then MessageLength field of CommunicateHeader would be dereferenced as a = physical address at OS runtime after SetVirtualAddressMap() and a page faul= t would occur.  My guess is that CommSize is never NULL, and that is why we have been lucky.

 

With the addition o= f the check for the V3 GUID, the HeaderGuid field of CommunicateHeader is a= lways dereferenced.  So if CommunicateHeader is a physical address at = OS Runtime after SetVirtualAddressMap(), then a page fault will always occur.  No more luck.=

 

Mike

 

From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, April 18, 2025 12:30 AM
To: Ni, Ray <ray.ni@i= ntel.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io= ; Ni, Ray <ray.ni@int= el.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

Hi Ray,

 

I will verify the code first thing tomorrow morning= . But I just looked at the code flow before the change, it seems that SmmCo= mmunicationMmCommunicate2 is also using physical address, and the common routine will deference the pointer to read message= length as well. I just checked variable runtime driver did not convert the= input physical pointer. Would that cause the same issue? Did I miss someth= ing or we have lucked out all along?

 

Regards,

Kun

 

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, April 17, 2025 11:45 PM
To: Kun Qin <Kun= .Qin@microsoft.com>
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io= ; Ni, Ray <ray.ni@int= el.com>
Subject: [EXTERNAL] A bug in the SmmCommunication V3 logic

 

Hi Qin,=

I think= there is a bug in the SmmCommunication protocol implementation.=

&n= bsp;

All 3 c= ommunication protocol calls go to the same communicate() function that test= s the HeaderGuid against the V3 GUID.

But whe= n the call is from runtime, reading the HeaderGuid using the physical addre= ss of communication buffer would cause page fault. The virtual address shou= ld be used.

The bug= was not there without your patch because the communicate routines happened= not to read any bytes from the communication buffer but simply pass the ad= dress to SMM. SMM expects the physical address because the virtual-to-physical mapping in SMM is identical.<= /o:p>

&n= bsp;

The bug= exists in both the SmmIpl.c in MdeModulePkg and the MmCommunicationDxe.c i= n StandaloneMmPkg.

The bug= would cause OS boot failure if there is any communication protocol invocat= ion after ExitBootService.

&n= bsp;

I guess= the bug might not be there in your first version of patch, but was introdu= ced when I asked you to consolidate the logic together.

&n= bsp;

Can you= kindly reproduce it locally and send out a fix after confirming?

&n= bsp;

Thanks,=

Ray

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#121271) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_CH3PR21MB4517E0C71DCFC81317721932E9BF2CH3PR21MB4517namp_--