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 7C76FD801B3 for ; Fri, 18 Apr 2025 22:23:00 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=Q/p2DnPxageOdyNwEWJkDFu4pUDC6woa/eh4yg3GUjQ=; 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=1745014980; v=1; x=1745274178; b=Oo8gb+0cvNnjyqE9m9M5sYkfm3Tjgcgvj+Sejj0Yf0jwtLJxW8auVTbJXspY9q2dhJau/cMn ZojXNS5VPBSfFKZg/s7COjU+OHN7IIrztAd1sc9Tv/tpXbwixLBhEfLiZLthNIwo9+kUmXCwhg4 l/UmE9De7QVVvoQJwn5rFRHicEg/YeHEdIVa+8GbvE9oqzoa5Lf/aXRUEwRsNiVmACKWn8g/gsy e3zFf8Cndxt8NeE7ZF3BcLoAiUHaCWIQbEXQZO/aDjGuQijYZDuRIJ+IMqX1NjWoyACYQ7ohtpO rw12cF+rwm5iNE948riQx51wK5Wd7Pk/I19y21uXM1tEQ== X-Received: by 127.0.0.2 with SMTP id yPxkYY7687511xFJwRcYbcLf; Fri, 18 Apr 2025 15:22:58 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) by mx.groups.io with SMTP id smtpd.web11.851.1745014977785739824 for ; Fri, 18 Apr 2025 15:22:57 -0700 X-CSE-ConnectionGUID: UyyyuiOxTnKh7DisSRiwYg== X-CSE-MsgGUID: 3IRVRQztTfmQ7Wk8WGp51w== X-IronPort-AV: E=McAfee;i="6700,10204,11407"; a="50480202" X-IronPort-AV: E=Sophos;i="6.15,222,1739865600"; d="scan'208,217";a="50480202" X-Received: from orviesa003.jf.intel.com ([10.64.159.143]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2025 15:22:57 -0700 X-CSE-ConnectionGUID: 7uSJd6BLTl2eUhtgmvGLKQ== X-CSE-MsgGUID: 8H4S2H9qT56Ylc7PPny29Q== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,222,1739865600"; d="scan'208,217";a="136090204" X-Received: from orsmsx901.amr.corp.intel.com ([10.22.229.23]) by orviesa003.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Apr 2025 15:22:58 -0700 X-Received: from ORSMSX901.amr.corp.intel.com (10.22.229.23) by ORSMSX901.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.14; Fri, 18 Apr 2025 15:22:56 -0700 X-Received: from orsedg603.ED.cps.intel.com (10.7.248.4) by ORSMSX901.amr.corp.intel.com (10.22.229.23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.14 via Frontend Transport; Fri, 18 Apr 2025 15:22:56 -0700 X-Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.48) by edgegateway.intel.com (134.134.137.100) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.44; Fri, 18 Apr 2025 15:22:56 -0700 X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com (2603:10b6:303:6d::19) by DM4PR11MB5311.namprd11.prod.outlook.com (2603:10b6:5:392::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.8655.31; Fri, 18 Apr 2025 22:22:37 +0000 X-Received: from CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::a886:6510:729d:f9d0]) by CO1PR11MB4929.namprd11.prod.outlook.com ([fe80::a886:6510:729d:f9d0%7]) with mapi id 15.20.8655.022; Fri, 18 Apr 2025 22:22:37 +0000 From: "Michael D Kinney via groups.io" To: Kun Qin , "Ni, Ray" CC: "devel@edk2.groups.io" , "Ni, Ray" , "Kinney, Michael D" Subject: Re: [edk2-devel] A bug in the SmmCommunication V3 logic Thread-Topic: A bug in the SmmCommunication V3 logic Thread-Index: AQHbsCxo5nQBH4RMRk+1qEepe02xqLOpBL4QgACO8lCAAComQIAAMy/AgAAC4LCAAAuzgA== Date: Fri, 18 Apr 2025 22:22:37 +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: CO1PR11MB4929:EE_|DM4PR11MB5311:EE_ x-ms-office365-filtering-correlation-id: b138bd19-0554-491e-7fde-08dd7ec78686 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam-message-info: =?us-ascii?Q?w0ZllEDphRVydWlYmIHo5BdJav2wty9tqGvWglXXsJLXJFIhk+Jqw5H081lL?= =?us-ascii?Q?cb6v8I8OnD43xHp7BUjAbiQy2zTrBHSIjWyVyd6e3QtB72poOrwY4lNATBWB?= =?us-ascii?Q?lHKG7vbsYhqDA/h1eFn81dd4q7vgm2cU7Lz3I6XCZZqD7n3rsGEm95Fk0QOk?= =?us-ascii?Q?HSlQIMG3erKXg3OSbMbeUtpMBWHuNvVClshnO6UZS2B7zMBLCLq3GEE39DOB?= =?us-ascii?Q?PQAoQZ9VSqbDrcbtSS4S5OEo+gulELmtIdMeagfgQfvZlytc0eQMyYRtoBlc?= =?us-ascii?Q?xorEoDRtMVjBKA1vvdV8HyFI/ErmTKCPnAoVKeeiest4VynNLKD+tN1dCOJM?= =?us-ascii?Q?3bVswJIotJUxjEE3wkQVhyqmgniAu4q73cJ761dAMpAo+GbbK2h62LoV8pf2?= =?us-ascii?Q?AdPbpfUlEauaqffgCQm1QXozVA6rstFSrFZwau4nF/Vp8GjvLNa4JmH7EiG7?= =?us-ascii?Q?5vKU7E2phPwFkVLG0CQAZXIlnqYinF4zL4VYmpchSGkznEOO4NrRYJl63GUU?= =?us-ascii?Q?m1yKrQvU/0t9v3yXhY25Lq6DRfi+xE3meVX8ynoYAM6oGpDW0kmAdd+vFGYO?= =?us-ascii?Q?qh5qBN9XoEL4PKDiQ6w9tdsPephrdZG0gwyo425g/+xTQaFi6yyVgDe1vwuz?= =?us-ascii?Q?PhvIeuJyPYwa1FrQsCg68u1BTyq18a6JdHm4SjmlaG5r5WX/S6VLmHIQe3d+?= =?us-ascii?Q?I8PsT8CQcQNqtXNCIVwMR3auocuuwY8JiAWwEmOzmqe6uXuX0Z5Ei6xTMTjj?= =?us-ascii?Q?q1UD+3t9gzGGhMUm8+xkVP6IxBEgb0qjS7c0pirwolPJ7G+QjR73e6PMX8sY?= =?us-ascii?Q?fBrjuJUTu1aznSPyRxgf7xF3FTGiz5Bvj/05Tw/zeg/Xz3XoCWCEyO6VI7DG?= =?us-ascii?Q?ohdiEvR9Rdq3q7ce0pcE6yB5O6+V1je0af7Hy2rBToB9qeZ+Gg0kJkSdo898?= =?us-ascii?Q?QiiXUj5dY4DRT5wbQgUBzAdY0KypzzhuTxN7rKfmgsX4HQ5gcvCos/+W4iUR?= =?us-ascii?Q?TgQZEXgKRk2aEJF9IJ7Ec5HilRF0u9g7g55bee/qZc4LQ2PNBXw2GtAqw3Pc?= =?us-ascii?Q?/s+hYwvx+2fzlhPYLeRzcb9t+25R4dzu2mZVT+sa8O/U2lnOeI0cUaX4DOGW?= =?us-ascii?Q?kHybNyjcpsRA3/DAaw9RgFZh2JLr3gjkyHsmSmbd5q4zRM7XCuDVAP2zkj+m?= =?us-ascii?Q?ptlE5mC0ci6iAGljIkCskAsU1y0DI2j9et74LnIu9Nf3ad7i0sqoFiVg+oDT?= =?us-ascii?Q?kkEprJo23NEDl+mwGYNsomDnns9H+bcokwmBWF1jvCoChCsdF8AbdOyB0a1B?= =?us-ascii?Q?PgiZT12/dsaCwHY6Dyk72jKZ6ajPyqwtlXLyg2rmxBwTMDDdv3uiH8qqBmXX?= =?us-ascii?Q?qPoE+7tWWsWndGsOd2iAqJ4EJbKdCYhpBu95Zm9hUzxF0r8XmvstdOD8+QLL?= =?us-ascii?Q?d5h3FQuGqvCk67QmV9MrN2IoKyx0rakoajJtf3ilovUDIcGymNZ1zuLFasJi?= =?us-ascii?Q?gfwsvsuxvkpW7wQ=3D?= x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?jcrlnhMwWwwukSD7dKj+Hw2L0kiTQlsq70eXBrw0OE3LD7V1eIh2oobLst64?= =?us-ascii?Q?aiL+sk73Vh0TwpgNMX+pPhvZNxIDkGKiDeht6NEc361Rl4qdiO1tzqwiaURL?= =?us-ascii?Q?l4qD/V+5f5eljAoCi5CRugTD3mAC3JLABCgmPU9aXcpf+bN/8hyGDnmxLQxh?= =?us-ascii?Q?DqS3zs6HD0IVEvveV+t41wpQqdNDHAo2Cb0cVDwGmf09DmsrqwEzJDHVenuq?= =?us-ascii?Q?GvfJobWTXDNkB4d0V4ex0TBzJBUbuFqIma3pI95XOICJ31hb+2r4EVOApqMV?= =?us-ascii?Q?7mtERMEJO5zy3f2M3swd5R0FoQ2OhW0nEiYQ9Wl5DgmM/VE9GNOFrL/aZiuk?= =?us-ascii?Q?PjwksTBHpTbX5ZnH3mxY/GttuC6VbygmruOTb0Z+Tgp7xioMEbsJBDeOjq1t?= =?us-ascii?Q?WHSfeGbLM+i7rGWukEBYoaV+F+iTabp55gRKU2yitzuS0zGg5mw8GSXxJ74A?= =?us-ascii?Q?AhHFoqWh0cXptM7rdSP0/ELSDjG0QMX4yIZ4EUdws5285FwH2/9zZWrHR8G3?= =?us-ascii?Q?ZeivyQqay0WDW14rk00nCpg5jo1Ju+WD90eS6YCB60wSYWVOSVmBmAZMT/4B?= =?us-ascii?Q?iieyytd5qiEkuC8/3ruuG6pA1QDPFuCGDNeXPDnbitIP+h+RSXkR4ocPKdgL?= =?us-ascii?Q?S+gYAVS/Vjg9PNlC1ZmCEQ3Y5R2l2/0F/3dg6vL6/waSljaTwig7tik0xt2E?= =?us-ascii?Q?i4dSkenU86ZStny1O+b1MFoLyRNYnX8+ULmvUaOYrdx+/Cc4pfDABE/jaQdx?= =?us-ascii?Q?/NKfpPuR5NsLKevPfvzFuXa2VIGTYmszZ8IAywsJWryfuz2gBreHcBno1kiU?= =?us-ascii?Q?6nYDMSXoXZIjCbTjOX3XY1NxpDicgEJeU0IG6PawprS4lOR/IYGJvvKK1+38?= =?us-ascii?Q?wSqcXces82jBph467b47Pv0EOR0E9UoUkkVqSbhdIzCJuRFWiU5Rg326hu0j?= =?us-ascii?Q?eAY1djtJTT/mgxrTUWrPs5gpAifkppw1K9/aT8RUqXXVipFCKZEtKizAH+cr?= =?us-ascii?Q?NeEPrBgIFdPwmtV7M4RaKYNzz51A4X5U11WYeiLVq1Hsu2S58kqed0Zmsk7e?= =?us-ascii?Q?RRVCxcOTKcJ/L4p/agdYzfZ0FCg6U5zBZlywV21R1ccEjsBVpAqRnqPmVwJp?= =?us-ascii?Q?Ihd5B6z398xnP1+TUa5xBNBqDC6Ucv0UNM8wmR5RIXcKQYGLf9dq6K+2m85k?= =?us-ascii?Q?Mic8ZkHgqRUrJUwgci6bs6Sf4FMQ4XiS8EKu99SkxadZoSCekyYPkf7F8VzY?= =?us-ascii?Q?T3GoTExklDmc3rCCEpedPuydePI/oXvDH7C4koW3g/zxz1ixVtw2xLFD9YIB?= =?us-ascii?Q?/rhz8TdFztbyOYljVBlcCtBVA1panqPhdQ7LpCsBMcSgE4xvVXr4ZPABmLND?= =?us-ascii?Q?yPJWbtmt0WFxVnUKtvHQeByEWvXhPosKY9FaCYcXBnKgGP+cMTwxyOc0QTSb?= =?us-ascii?Q?yYbN548lkNmcYSo2TUvc/SUY3yge5rW0cILC9b7iSVnY2MH3loZPYj8ARoLR?= =?us-ascii?Q?cfyL1NMaT99SMnurOOjNGVcGk4bcDqecw6A8oPrAIpJZA5zLFkf7UqkmC4O/?= =?us-ascii?Q?aRHZpQHm+ciWBllb7sOH/KbbfSjm9LdqBNFCa6yUz8rsnVAjNqCTEyoh6jQV?= =?us-ascii?Q?OA=3D=3D?= MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4929.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: b138bd19-0554-491e-7fde-08dd7ec78686 X-MS-Exchange-CrossTenant-originalarrivaltime: 18 Apr 2025 22:22:37.6768 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: CNQDRhxd2d+1a4MV4iGCV6TLxdnXv53DwmfmGi6FkwzVChbgC4N2MUJDODZFyWELaHoKInw/obwsP+o38z6GBmMD287ev4WeOXm+vpARjJI= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5311 X-OriginatorOrg: intel.com 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 15:22:57 -0700 Resent-From: michael.d.kinney@intel.com Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 23L0EIb8QL0EE6Sjx4KK07Y7x7686176AA= Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_CO1PR11MB492933149BDB7669A2EDA9E2D2BF2CO1PR11MB4929namp_" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240830 header.b=Oo8gb+0c; 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_CO1PR11MB492933149BDB7669A2EDA9E2D2BF2CO1PR11MB4929namp_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I think my proposal is better to have separate logic. One for V1/V2 and an= other for V3. I agree that V1 must pass in non NULL CommSize and we can do spec change fo= r that since NULL CommSize is not implementable. V2 can support CommSize optional because it has phys and virt. V3 does not have CommSize. Mike From: Kun Qin Sent: Friday, April 18, 2025 3:18 PM To: Kinney, Michael D ; Ni, Ray Cc: devel@edk2.groups.io; Ni, Ray Subject: RE: A bug in the SmmCommunication V3 logic 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 (#121268): https://edk2.groups.io/g/devel/message/121268 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_CO1PR11MB492933149BDB7669A2EDA9E2D2BF2CO1PR11MB4929namp_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

I think my proposal= is better to have separate logic.  One for V1/V2 and another for V3.<= o:p>

 

I agree that V1 mus= t pass in non NULL CommSize and we can do spec change for that since NULL C= ommSize is not implementable.

 

V2 can support Comm= Size optional because it has phys and virt. 

 

V3 does not have Co= mmSize.

 

Mike

 

 

 

From: Kun Qin <Kun.Qin@microsoft.= com>
Sent: Friday, April 18, 2025 3:18 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ray &l= t;ray.ni@intel.com>
Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Subject: RE: A bug in the SmmCommunication V3 logic

 

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@mic= rosoft.com>; Ni, Ray <ray.ni@= intel.com>
Cc: devel@edk2.groups.io= ; Ni, Ray <ray.ni@intel.com>;= Kinney, Michael D <michae= l.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 (#121268) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--_000_CO1PR11MB492933149BDB7669A2EDA9E2D2BF2CO1PR11MB4929namp_--