[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH for-4.16 v2] efi: fix alignment of function parameters in compat mode
- To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Roger Pau Monne <roger.pau@xxxxxxxxxx>
- Date: Thu, 18 Nov 2021 09:28:06 +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=7iI4sG5ccFDAseKHYLIAbbrV1FZm8Jj/ahp9xsvOjKA=; b=FfaHf26vRCWOGaE/p20/C4Dyazje2sUbh0e7+U4psu2rqx/Q8d9ZncLSWL30eis5QjjmpBarpRMvrPLAJTFy6HAICUKFOLXrmiS8bo9OLPKgSDimBa12u9Uf1aUglh/xHXm3HD7A3t7EHVzOb1RpG0ZQ4oMTYhXalfJvohq0FJQzYbqpDoI+EtZZUZ/gmEQDprY7+yo/Q9sl4B9mzVZCYBuGWUjNx+rlKd1gW/ftGxk5belM4hLUYgqOPMI1kLcqioYJADqVq1ocJ5NjA7VZrXKpD7bPeUFJDdnG29Hd/rlH5MPGmKZAsp4dJN/M8g0Y2Kzvf2o+9zSLOrniFwJdtg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mJ9DEGfjAiUMnAUwZP1pcwKCggI+3d0WuFYKNN5QcEow5Tck3RKWZwCk2C9PVIAIqh7/wL/jEn0QZgjPo6JQdxD3ZMz426LBYD6eZ+353MH9cJGgp2YI5rdJhWd+VWc66G0oev/9fNUuWSA8dZLsQfcRnpCNzse2DkQfw06Qa0KpKl83iLoWcaqGjFN/IAKjdSuwUDuKKyi1a1VQyK70CrtwfVKqxAsHB4OY2ovNb9Vxo6aBeCZRQ0bEP73Z3O/Y99MC4VX8CrS2cgChVdNgoFFyuJfkPwx6eNTnvf2mGqanVB30A4zqLHAC1cGp/BUFo7GPSrqelxFuMdm6hz29/w==
- Authentication-results: esa4.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: Thu, 18 Nov 2021 08:28:33 +0000
- Ironport-data: A9a23:HtaJyqI88ayM06inFE+RNpIlxSXFcZb7ZxGr2PjKsXjdYENShDwBz mEWWTqGbKyCNGT3Lop0a4i1ox5Q7JPRzIRrS1BlqX01Q3x08seUXt7xwmUcns+xwm8vaGo9s q3yv/GZdJhcokcxIn5BC5C5xZVG/fjgqoHUVaiUZUideSc+EH140Es6y7Zg6mJVqYPR7z2l6 IuaT/L3YDdJ6xYsWo7Dw/vewP/HlK2aVAIw5jTSV9gS1LPtvyB94KYkDbOwNxPFrrx8RYZWc QphIIaRpQs19z91Yj+sfy2SnkciGtY+NiDW4pZatjTLbrGvaUXe345iXMfwZ3u7hB2EsNdd1 ucdsaWzF1d0P6btvNQeeB5HRnQW0a1uoNcrIFC6uM2XiUbHb2Ht07NlC0Re0Y8wo7gtRzsUr LpBdW5LPkvra+GemdpXTsF2gcsuNo/zNZ43sXB81zDJS/0hRPgvRo2UvYcIjWxr2aiiG97sT ZQfdwJpdC7gPUdNMxAUIpA1p+qB0yyXnzpw9wvO+PtfD3Lo5A1u0pD9PdzNYNuISM5J2EGCq Qru/W70HxUbP9y30iee/zSngeqntTP2XsceGaO18tZugUaP3SoDBRsOT1y5rPKlzEmkVLpix 1c8o3R06/JorQryE4e7D0bQTGO4UgA0VvVXP/MIuAW0yfSEvgeSJ0w0TRVGd4lz3CMpfgAC2 liMltLvIDVgtryJVH6QnoupQSOO1Ts9djFbO3JdJecRy5y6+dxo0EqTJjp2OPft1oWdJN3m/ 9ydQMHSbZ03hNVD6ai09Euvb9mE9smQFV5dCuk6swuYAuJFiGyNO9zABbvzt68owGOlor+p5 iRsdy+2tr5mMH11vHbRKNjh5Znwjxp/DBXSgER0A74q/Cm39niocOh4uW8lex42bptYKWK5O ic/XD+9ArcJYBNGioctPeqM5zkCl/C8RbwJqNiKBjaxXnSBXFDep3w/DaJh92vsjFItgckC1 WSzKq6R4YIhIf0/llKeHr5FuZdyn3xW7T6DFPjTkkX8uZLDNSH9dFvwGAbXBgzPxPjf+1u9H hc2H5bi9iizp8WiOHSKqtBKcghRRZX5bLivw/Fqmie4ClMOMEkqCuPLwKNnfIpgnq9PkfzP8 G37UUhdoGcTT1WeQelTQnw8Or7pQ7hlqnc3YX4lMVqygiBxaoez9qYPMZAweOB/puBkyPd1S dgDetmBXasTGmiWpWxFYMmvtpFmeTSqmRmKY3ivbg8gcsMyXAfO4NLlIFfirXFcEiqtuMIii LS8zQeHE4EbTgFvAZ+OOvKixl+8p1YHn+d2UxeaK9VfYhy0ooNrNzbwnrk8JMRVcUfPwT6T1 gC3BxYEpLaS/99poYeR3a3d9tWnCepzGEZeDlL317fuOHmI5HenzK9BTP2MIWLXWlTr9fjwf u5S1fz9bqEKxQ4Yr4pmHr935qsi/N+z9aRCxwFpEXiXPVSmDrRsfiuP0cVV7/Afw7ZYvU29W 16V+8kcMrKMYZu3HFkULQsjT+KCyfBLxWWCsaVreB33tH1t4b6KcUROJB3d2iVSIYx8PJ4h3 ep86tUd7Bayi0ZyP9uL5syOG79g8pDUv30bi6wn
- Ironport-hdrordr: A9a23:HtqupqmuKeMeRylBaz4x5y/oRfjpDfPCimdD5ihNYBxZY6Wkfp +V88jzhCWZtN9OYhwdcLC7WZVpQRvnhPpICO4qTM2ftWjdyRCVxeRZg7cKrAeQfREWmtQtt5 uIEJIOd+EYb2IK9PoSiTPQe71LoKjlgd6VbI/lvgtQpGpRGsZdBmlCe2Om+hocfng6OXN1Lu vV2uN34x6bPVgHZMWyAXcIG8DFut3wjZrjJToLHQQu5gWihS6hrOeSKWnT4j4uFxd0hZsy+2 nMlAL0oo2lrvGA0xfZk0ve9Y5fltfNwsZKQOaMls8WADPxjRvAXvUrZ5Sy+BQO5M2/4lcjl9 fB5z8mIsRI8nvUOlq4pBP8sjOQpwoG2jvH8xu1kHHjqcv2SHYREMxan79UdRPf9g4JoMx8+L gj5RPXi7NnSTf72Ajt7dnBUB9n0mCup2A5rOIVh3tDFaMDdb5qq5AF9k89KuZNIMvD0vFnLA BSNrCd2B4PGmnqLEwx/1MfjeBEZ05DUCtvGSM5y46oOzs/pgEM86JX/r1bop46zuNMd3Bz3Z WwDk1ZrsA+ciYoV9MPOA54e7rONoXse2O7DIvAGyWvKEk4U0i92aIfpo9FoN2XRA==
- Ironport-sdr: KwRLzz7fyEzqornLgetH9Q+NvIYLi0hbFwU1G41ue/BcYgi7Xtd9cGDvyGPvjYGFos4VtaXkKc 8z0aYINN8Ch2+4qhdUd++OgdJf3EcXEcEnEsuvQWnTGc7nI7chT/7myMNxm+NPyygEk2fdmu+f MO0CSVFwOjR7xHidfaTJzS5NT4ufHJLmuBdPPQJfy7HopmKZWL9G0GhC9SmtsIfMtXJRJ2LkNs VJZAq6xJsHQX23jbIjrU09AdmeAQRS9XUKSUsrh+GQBe0FjllSpB/0bMx58ymIcTCR1vTiHGnu OBRiVT6OTmeKY2ZEh08KhaFc
- 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
13.0.0 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>
Release-Acked-by: Ian Jackson <iwj@xxxxxxxxxxxxxx>
---
Changes since v1:
- Copy back the results.
---
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 | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index 375b94229e..f1cbf3088c 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,34 @@ 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);
+
+ op->u.query_variable_info.max_store_size = max_store_size;
+ op->u.query_variable_info.remain_store_size = remain_store_size;
+ op->u.query_variable_info.max_size = max_size;
+
break;
+ }
case XEN_EFI_query_capsule_capabilities:
case XEN_EFI_update_capsule:
--
2.33.0
|