)]}' { "commit": "70d54d4a6e1cd55db4d9ec00e746e79e98493226", "tree": "da9da41f1bfba2aa1efd781ae8d149995aaf158f", "parents": [ "a2e47087551b55145c4229053343d2402060ad84" ], "author": { "name": "Hao Wu", "email": "hao.a.wu@intel.com", "time": "Thu Sep 13 15:47:10 2018 +0800" }, "committer": { "name": "Hao Wu", "email": "hao.a.wu@intel.com", "time": "Fri Nov 16 09:00:05 2018 +0800" }, "message": "MdeModulePkg/Variable: [CVE-2017-5753] Fix bounds check bypass\n\nREF:https://bugzilla.tianocore.org/show_bug.cgi?id\u003d1194\n\nSpeculative execution is used by processor to avoid having to wait for\ndata to arrive from memory, or for previous operations to finish, the\nprocessor may speculate as to what will be executed.\n\nIf the speculation is incorrect, the speculatively executed instructions\nmight leave hints such as which memory locations have been brought into\ncache. Malicious actors can use the bounds check bypass method (code\ngadgets with controlled external inputs) to infer data values that have\nbeen used in speculative operations to reveal secrets which should not\notherwise be accessed.\n\nThis commit will focus on the SMI handler(s) registered within the\nVariable\\RuntimeDxe driver and insert AsmLfence API to mitigate the\nbounds check bypass issue.\n\nFor SMI handler SmmVariableHandler():\n\nUnder \"case SMM_VARIABLE_FUNCTION_GET_VARIABLE:\",\n\u0027SmmVariableHeader-\u003eNameSize\u0027 can be a potential cross boundary access of\nthe \u0027CommBuffer\u0027 (controlled external input) during speculative execution.\n\nThis cross boundary access is later used as the index to access array\n\u0027SmmVariableHeader-\u003eName\u0027 by code:\n\"SmmVariableHeader-\u003eName[SmmVariableHeader-\u003eNameSize/sizeof (CHAR16) - 1]\"\nOne can observe which part of the content within array was brought into\ncache to possibly reveal the value of \u0027SmmVariableHeader-\u003eNameSize\u0027.\n\nHence, this commit adds a AsmLfence() after the boundary/range checks of\n\u0027CommBuffer\u0027 to prevent the speculative execution.\n\nAnd there are 2 similar cases under\n\"case SMM_VARIABLE_FUNCTION_SET_VARIABLE:\" and\n\"case SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET:\" as well.\nThis commits also handles them.\n\nAlso, under \"case SMM_VARIABLE_FUNCTION_SET_VARIABLE:\",\n\u0027(UINT8 *)SmmVariableHeader-\u003eName + SmmVariableHeader-\u003eNameSize\u0027 points to\nthe \u0027CommBuffer\u0027 (with some offset) and then passed as parameter \u0027Data\u0027 to\nfunction VariableServiceSetVariable().\n\nWithin function VariableServiceSetVariable(), there is a sanity check for\nEFI_VARIABLE_AUTHENTICATION_2 descriptor for the data pointed by \u0027Data\u0027.\nIf this check is speculatively bypassed, potential cross-boundary data\naccess for \u0027Data\u0027 is possible to be revealed via the below function calls\nsequence during speculative execution:\n\nAuthVariableLibProcessVariable()\nProcessVarWithPk() or ProcessVarWithKek()\n\nWithin function ProcessVarWithPk() or ProcessVarWithKek(), for the code\n\"PayloadSize \u003d DataSize - AUTHINFO2_SIZE (Data);\", \u0027AUTHINFO2_SIZE (Data)\u0027\ncan be a cross boundary access during speculative execution.\n\nThen, \u0027PayloadSize\u0027 is possible to be revealed by the function call\nsequence:\n\nAuthServiceInternalUpdateVariableWithTimeStamp()\nmAuthVarLibContextIn-\u003eUpdateVariable()\nVariableExLibUpdateVariable()\nUpdateVariable()\nCopyMem()\n\nHence, this commit adds a AsmLfence() after the sanity check for\nEFI_VARIABLE_AUTHENTICATION_2 descriptor upon \u0027Data\u0027 within function\nVariableServiceSetVariable() to prevent the speculative execution.\n\nAlso, please note that the change made within function\nVariableServiceSetVariable() will affect DXE as well. However, since we\nonly focuses on the SMM codes, the commit will introduce a new module\ninternal function called VariableLoadFence() to handle this. This internal\nfunction will have 2 implementations (1 for SMM, 1 for DXE). For the SMM\nimplementation, it is a wrapper to call the AsmLfence() API; for the DXE\nimplementation, it is empty.\n\nA more detailed explanation of the purpose of commit is under the\n\u0027Bounds check bypass mitigation\u0027 section of the below link:\nhttps://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation\n\nAnd the document at:\nhttps://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf\n\nCc: Jiewen Yao \u003cjiewen.yao@intel.com\u003e\nContributed-under: TianoCore Contribution Agreement 1.1\nSigned-off-by: Hao Wu \u003chao.a.wu@intel.com\u003e\nReviewed-by: Star Zeng \u003cstar.zeng@intel.com\u003e\nAcked-by: Laszlo Ersek \u003clersek@redhat.com\u003e\nRegression-tested-by: Laszlo Ersek \u003clersek@redhat.com\u003e\n(cherry picked from commit e83d841fdc2878959185c4c6cc38a7a1e88377a4)\n", "tree_diff": [ { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "0f64ee093b04ff5176156635326d39865d032150", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceDxe.c" }, { "type": "add", "old_id": "0000000000000000000000000000000000000000", "old_mode": 0, "old_path": "/dev/null", "new_id": "4b0d7e3e95610167ec4f420797ec3769350d6209", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/LoadFenceSmm.c" }, { "type": "modify", "old_id": "b98b8556a23a96361da84a6c37176b9e6765b817", "old_mode": 33188, "old_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h", "new_id": "a324ad23650fbf6f5ccce01b6072aab2beea053f", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h" }, { "type": "modify", "old_id": "905e5e049930190a699c041d1edb0c1a26210523", "old_mode": 33188, "old_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", "new_id": "93ac461f4d0b262ff4eccd4c7ded4c588808a1a6", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c" }, { "type": "modify", "old_id": "e840fc9bff40fd69f6901b87301a5cb4706bfd65", "old_mode": 33188, "old_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf", "new_id": "42fbb5cfee339e81310392a5f9038c1488aa16f4", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf" }, { "type": "modify", "old_id": "8d73b6edee5118f2a14ae1df685a3327daa55fae", "old_mode": 33188, "old_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c", "new_id": "d1875f1425cc242170e3c8ee7eb8232453570c3f", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c" }, { "type": "modify", "old_id": "69966f0d37ee3f400d8760fd289bf232e7f61476", "old_mode": 33188, "old_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf", "new_id": "65c163795fc0ba3c0a653db0c504c8136cac7178", "new_mode": 33188, "new_path": "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf" } ] }