• Reinette Chatre's avatar
    x86/sgx: Disconnect backing page references from dirty status · 6bd42964
    Reinette Chatre authored
    SGX uses shmem backing storage to store encrypted enclave pages
    and their crypto metadata when enclave pages are moved out of
    enclave memory. Two shmem backing storage pages are associated with
    each enclave page - one backing page to contain the encrypted
    enclave page data and one backing page (shared by a few
    enclave pages) to contain the crypto metadata used by the
    processor to verify the enclave page when it is loaded back into
    the enclave.
    
    sgx_encl_put_backing() is used to release references to the
    backing storage and, optionally, mark both backing store pages
    as dirty.
    
    Managing references and dirty status together in this way results
    in both backing store pages marked as dirty, even if only one of
    the backing store pages are changed.
    
    Additionally, waiting until the page reference is dropped to set
    the page dirty risks a race with the page fault handler that
    may load outdated data into the enclave when a page is faulted
    right after it is reclaimed.
    
    Consider what happens if the reclaimer writes a page to the backing
    store and the page is immediately faulted back, before the reclaimer
    is able to set the dirty bit of the page:
    
    sgx_reclaim_pages() {                    sgx_vma_fault() {
      ...
      sgx_encl_get_backing();
      ...                                      ...
      sgx_reclaimer_write() {
        mutex_lock(&encl->lock);
        /* Write data to backing store */
        mutex_unlock(&encl->lock);
      }
                                               mutex_lock(&encl->lock);
                                               __sgx_encl_eldu() {
                                                 ...
                                                 /*
                                                  * Enclave backing store
                                                  * page not released
                                                  * nor marked dirty -
                                                  * contents may not be
                                                  * up to date.
                                                  */
                                                  sgx_encl_get_backing();
                                                  ...
                                                  /*
                                                   * Enclave data restored
                                                   * from backing store
                                                   * and PCMD pages that
                                                   * are not up to date.
                                                   * ENCLS[ELDU] faults
                                                   * because of MAC or PCMD
                                                   * checking failure.
                                                   */
                                                   sgx_encl_put_backing();
                                                }
                                                ...
      /* set page dirty */
      sgx_encl_put_backing();
      ...
                                                mutex_unlock(&encl->lock);
    }                                        }
    
    Remove the option to sgx_encl_put_backing() to set the backing
    pages as dirty and set the needed pages as dirty right after
    receiving important data while enclave mutex is held. This ensures that
    the page fault handler can get up to date data from a page and prepares
    the code for a following change where only one of the backing pages
    need to be marked as dirty.
    
    Cc: stable@vger.kernel.org
    Fixes: 1728ab54 ("x86/sgx: Add a page reclaimer")
    Suggested-by: default avatarDave Hansen <dave.hansen@linux.intel.com>
    Signed-off-by: default avatarReinette Chatre <reinette.chatre@intel.com>
    Signed-off-by: default avatarDave Hansen <dave.hansen@linux.intel.com>
    Reviewed-by: default avatarJarkko Sakkinen <jarkko@kernel.org>
    Tested-by: default avatarHaitao Huang <haitao.huang@intel.com>
    Link: https://lore.kernel.org/linux-sgx/8922e48f-6646-c7cc-6393-7c78dcf23d23@intel.com/
    Link: https://lkml.kernel.org/r/fa9f98986923f43e72ef4c6702a50b2a0b3c42e3.1652389823.git.reinette.chatre@intel.com
    6bd42964
encl.h 2.95 KB