[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH for-4.16] efi: fix alignment of function parameters in compat mode


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
  • Date: Wed, 17 Nov 2021 16:33:50 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Z7yVKsAWCWZHBToll3wYcVdUhfwMwV/kcO/6YqeI6j4=; b=ImngCNqYCptTAPdyCpwqeNMzcGmzWD+y1g4u1m1yK8DxY1hOhxz5z4QS1bNhPmfRg2uZyN0it4ZvYjueH03mdmxN7gTSXR+5X+1TUiG0zE26EvoPuHKGefbxBfT9yYsPWzQ0rqKqzrcHmnIA0Lje5v2Xf1Xw4ISm0ISkicytmIyfGa/pJWohLwfkFC8LAKLHqC8ruqV0L8MjbtDvMtACOni1wSDgnJg+5d8WBGcLjv73oLTaGPgSNdgSxDiiZp96I6jaUtwOc8X4SORs4tpQGs1dmB1brW2GhE5gZfo7NjQ3fErKuk8E/VRV5JrywcmFAvHjijrRNJOnL9EYBq4bFg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=lImEgYsx8mrSNqxVwXcDauW7mGp0yxrxSnodxUud1mzPfgtI056UYQv0n2VrEKnN+s/6g+sDuFGVBbgIWJfXYCS+0DE5sil1pt0ELknP6SFxgO81u5XDJ3f7fvnzTiMz+oSdirRd2G6vYzziPGCKnNL1MNh8uHHeiSHKvXV8lhu2gIHgdmkjz89lbFFwKjjmpADj93/YQ6kO2AuBnUInn+9JKhoFxvqHY+LjIVx/yutZIeeQoN1NvoQ891ADRzF4PReKqFqCuKZ55qVw5lq8nvYDDdMuUEp+99hvzLqXdDGEg41i4DoPQeP38JHokvyroBVPtUlh+KDOKANaDlCXBw==
  • Authentication-results: esa6.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>
  • Delivery-date: Wed, 17 Nov 2021 15:34:43 +0000
  • Ironport-data: A9a23:Qrh0uqK+DXg9bu/gFE+RNpIlxSXFcZb7ZxGr2PjKsXjdYENS1zEAy mtLXDiGP/+NMGr3f4xyOY238EtT7ZXRxoBgHVdlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokcxIn5BC5C5xZVG/fjgqoHUVaiUZUideSc+EH140Es6xLZi6mJVqYPR7z2l6 IuaT/L3YDdJ6xYsWo7Dw/vewP/HlK2aVAIw5jTSV9gS1LPtvyB94KYkDbOwNxPFrrx8RYZWc QphIIaRpQs19z91Yj+sfy2SnkciGtY+NiDW4pZatjTLbrGvaUXe345iXMfwZ3u7hB2nvc9e2 MQT8qWhYkRxPPHLnu0sfD5XRnQW0a1uoNcrIFC6uM2XiUbHb2Ht07NlC0Re0Y8wo7gtRzsUr LpBdW5LPkvra+GemdpXTsF2gcsuNo/zNZ43sXB81zDJS/0hRPgvRo2XtYcAhmlq26iiG97eY sQIcWZWdyjKXCB1IUsRKpUn28aB0yyXnzpw9wvO+PtfD3Lo5A1u0pD9PdzNYNuISM5J2EGCq Qru/W70HxUbP9y30iee/zSngeqntTP2XsceGaO18tZugUaP3SoDBRsOT1y5rPKlzEmkVLpix 1c8o3R06/JorQryE4e7D0bQTGO4UgA0dfhPPP0rtka024GXuwTFAW4NfxFmZ4lz3CMpfgAC2 liMltLvIDVgtryJVH6QnoupQSOO1Ts9djFbO3JdJecRy5y6+dxo0EqTJjp2OPft1oWdJN3m/ 9ydQMHSbZ03hNVD6ai09Euvb9mE9smQFV5dCuk6swuYAuJFiGyNO9zABbvzt68owGOlor+p5 iBsdy+2tr5mMH11vHbRKNjh5Znwjxp/DBXSgER0A74q/Cm39niocOh4uW8lex85bZpbJWW4P Sc/XD+9ArcJZxNGioctPeqM5zkCl/C8RbwJqNiKBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzKq6R4YIhIf0/llKeHr5FuZdyn3xW7T6DFPjTkkX8uZLDNSH9dFvwGAbXBgzPxPjf+1u9H hc2H5bi9iizp8WiOHSKqtBKcghRRZX5bLivw/Fqmie4ClMOMEkqCuPLwKNnfIpgnq9PkfzP8 G37UUhdoGcTT1WcQelTQnw8Or7pQ7hlqnc3YX4lMVqygiBxaoez9qYPMZAweOB/puBkyPd1S dgDetmBXasTGmiWpWxFYMmvtpFmeTSqmRmKY3ivbg8gcsMyXAfO4NLlIFfirXFcEiqtuMIii LS8zQeHE4EbTgFvAZ+OOvKixl+8p1YHn+d2UxeaK9VfYhy0ooNrNzbwnrk8JMRVcUfPwT6T1 gC3BxYEpLaS/99poYeR3a3d9tWnCepzGEZeDlL317fuOHmI5HenzK9BTP2MIWLXWlTr9fjwf u5S1fz9bqEKxQ4Yr4pmHr935qsi/N+z9aRCxwFpEXiXPVSmDrRsfiuP0cVV7/Afw7ZYvU29W 16V+8kcMrKMYZu3HFkULQsjT+KCyfBLxWWCsaVreB33tH1t4b6KcUROJB3d2iVSIYx8PJ4h3 ep86tUd7Bayi0ZyP9uL5syOG79g8pDUv30bi6wn
  • Ironport-hdrordr: A9a23:m52WsKGDxNb9EqoApLqE7MeALOsnbusQ8zAXPidKOHtom62j5q STdZEgviMc5wx8ZJhNo7+90cq7IU80l6Qa3WB5B97LNmTbUQCTTb1K3M/PxCDhBj271sM179 YET0GmMqySMbGtt7eZ3DWF
  • Ironport-sdr: 4JKZ5GC3ea4Rvfdd+9Wbt53YAzJne3nl7H9MoUFrhfFY48ZwHU7VJoQ7VeV8rQGPl+j8uyQ4O4 0sPTlu0OIbEazg1oml6v87NSIUXcb6kJVQe34+5fdjlCSNUuA8Wgfds62oDnG97lSX/UDdkasc 4PfU7Z70+jtkTtP/jCh0GUEge4OVsdT8abRw7EgB3Ij5OPD1EjL2rMXznq3ZQGBaUuMuQqPSa6 4qqbHrgzceKYjldn1kEl8A1c9glPVgBWZwmC+FsD4I0ZoA9J5gDDoEfYcIrGrxlLN0vBqxHYBk CgJdGBzovTDwJRhf6jhKvDnu
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Currently the max_store_size, remain_store_size and max_size in
compat_pf_efi_runtime_call are 4 byte aligned, which makes clang
complain with:

In file included from compat.c:30:
./runtime.c:646:13: error: passing 4-byte aligned argument to 8-byte aligned 
parameter 2 of 'QueryVariableInfo' may result in an unaligned pointer access 
[-Werror,-Walign-mismatch]
            &op->u.query_variable_info.max_store_size,
            ^
./runtime.c:647:13: error: passing 4-byte aligned argument to 8-byte aligned 
parameter 3 of 'QueryVariableInfo' may result in an unaligned pointer access 
[-Werror,-Walign-mismatch]
            &op->u.query_variable_info.remain_store_size,
            ^
./runtime.c:648:13: error: passing 4-byte aligned argument to 8-byte aligned 
parameter 4 of 'QueryVariableInfo' may result in an unaligned pointer access 
[-Werror,-Walign-mismatch]
            &op->u.query_variable_info.max_size);
            ^
Fix this by bouncing the variables on the stack in order for them to
be 8 byte aligned.

Note this could be done in a more selective manner to only apply to
compat code calls, but given the overhead of making an EFI call doing
an extra copy of 3 variables doesn't seem to warrant the special
casing.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
Cc: Ian Jackson <iwj@xxxxxxxxxxxxxx>

Tagged for possible inclusion into the release, as it's a likely
candidate for backport. It shouldn't introduce any functional change
from a caller PoV. I think the risk is getting the patch wrong and not
passing the right parameters, or broken EFI implementations corrupting
data on our stack instead of doing it in xenpf_efi_runtime_call.
---
 xen/common/efi/runtime.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 375b94229e..4462233798 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -607,6 +607,9 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
     break;
 
     case XEN_EFI_query_variable_info:
+    {
+        uint64_t max_store_size, remain_store_size, max_size;
+
         if ( op->misc & ~XEN_EFI_VARINFO_BOOT_SNAPSHOT )
             return -EINVAL;
 
@@ -638,16 +641,29 @@ int efi_runtime_call(struct xenpf_efi_runtime_call *op)
 
         if ( !efi_enabled(EFI_RS) || (efi_rs->Hdr.Revision >> 16) < 2 )
             return -EOPNOTSUPP;
+
+        /*
+         * Bounce the variables onto the stack to make them 8 byte aligned when
+         * called from the compat handler, as their placement in
+         * compat_pf_efi_runtime_call will make them 4 byte aligned instead and
+         * clang will complain.
+         *
+         * Note we do this regardless of whether called from the compat handler
+         * or not, as it's not worth the extra logic to differentiate.
+         */
+        max_store_size = op->u.query_variable_info.max_store_size;
+        remain_store_size = op->u.query_variable_info.remain_store_size;
+        max_size = op->u.query_variable_info.max_size;
+
         state = efi_rs_enter();
         if ( !state.cr3 )
             return -EOPNOTSUPP;
         status = efi_rs->QueryVariableInfo(
-            op->u.query_variable_info.attr,
-            &op->u.query_variable_info.max_store_size,
-            &op->u.query_variable_info.remain_store_size,
-            &op->u.query_variable_info.max_size);
+            op->u.query_variable_info.attr, &max_store_size, 
&remain_store_size,
+            &max_size);
         efi_rs_leave(&state);
         break;
+    }
 
     case XEN_EFI_query_capsule_capabilities:
     case XEN_EFI_update_capsule:
-- 
2.33.0




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.