Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp2469161pxj; Mon, 10 May 2021 03:51:59 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyl3kAwwOYNFmhzf7/xclsSjLgELQiEay3s1tKlt4/50oWlHG6hb7/lqmdHx1ujp5Dq3EnE X-Received: by 2002:a5d:91c6:: with SMTP id k6mr13649524ior.148.1620643919212; Mon, 10 May 2021 03:51:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1620643919; cv=none; d=google.com; s=arc-20160816; b=bI2a5W1v3Z7l7jeMEIbvKMbnqTWvYT4XXTcAtribQMFyN2vfHJEuRH+VypxtF5STpf 7/NB8g1ZggSwIsHPZxvRBXVTVbAjhJVe74fZaoUq97gIKvEc8u4xPunzkhyoiyaWSu0Y EOgOa3NAB2m+LyLxwbj/HOwynwUZ5vS3WqhhGafV2YUoixINtS8hWD11FZpkhdbDEXv4 Y36hZ9U6ZHEyITmdBHM0eIFXQqp6Nim6yrk11E1WZoiPis5IPhOe7SJvI8dAGp6ve26j BWoEMJFV0BGjyg+RiT6FPVEknWcHGlZsZwI6f1ACirmALVtZvH1+2asZ62/MwjrM5e9Q fYyg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=LFd2JYP+JDptmkVBDnHbp0QtmcK+4AYWPftv038o0Jk=; b=YAldd/b3FNFtMUcR4iC0uZqyByRUhzR50D+oMeq1wUrRXV+JmRKlQG0tCkLSMm1gPt EcKpMRtCJgSbwHs+BkZW+tAb2OqBQYB9LucMDwIRQ+ulUY8Xj+WDnKGtivBXx2qQ+T7a f+qZt8+tPLWPNV4qiNnDwZ7xg8D2TwJOcSUEEtPmsMmxHLk4lkEpj8ZjrFaI18VKXzrq sgBNU0pNdLcrpTOoXxX39EvIH6KMlJ0Hc5NAZphLzqHo4ykEPrWcm4V9bDQRDkYqKmW2 JHSSj3o/bxdSofd3HVEQqRbN0IwdUuxIEKghyjiWX+mRbG0ZEzgXH0h12TIz+d4cojfY riuw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=oP9rPBiT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id u9si3518141jak.104.2021.05.10.03.51.42; Mon, 10 May 2021 03:51:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=oP9rPBiT; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233687AbhEJKub (ORCPT + 99 others); Mon, 10 May 2021 06:50:31 -0400 Received: from mail.kernel.org ([198.145.29.99]:52756 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232976AbhEJKky (ORCPT ); Mon, 10 May 2021 06:40:54 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 20F696191A; Mon, 10 May 2021 10:31:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1620642689; bh=DkoOxMJz8YpuchWQL/RtRgbH0rxYa/FDFDv2JcYcdZo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=oP9rPBiTWDAlIbHLxMTGn38LlLemBBs7C4GIjdyVS/Ie/5GN9IU1Xt9n9mjLsHhcU H0ItzIG3TV5QyAbLBJRjbci6xmBqxEO4EtdmAhMgetlwyMVCUQmfvLLpEC6EhDdpkg z48SCGQQeZpo0jSlzhpJ924xGg9oVuIRnqzMm9+4= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Mathias Krause , Andra Paraschiv Subject: [PATCH 5.10 004/299] nitro_enclaves: Fix stale file descriptors on failed usercopy Date: Mon, 10 May 2021 12:16:41 +0200 Message-Id: <20210510102004.973967515@linuxfoundation.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210510102004.821838356@linuxfoundation.org> References: <20210510102004.821838356@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Mathias Krause commit f1ce3986baa62cffc3c5be156994de87524bab99 upstream. A failing usercopy of the slot uid will lead to a stale entry in the file descriptor table as put_unused_fd() won't release it. This enables userland to refer to a dangling 'file' object through that still valid file descriptor, leading to all kinds of use-after-free exploitation scenarios. Exchanging put_unused_fd() for close_fd(), ksys_close() or alike won't solve the underlying issue, as the file descriptor might have been replaced in the meantime, e.g. via userland calling close() on it (leading to a NULL pointer dereference in the error handling code as 'fget(enclave_fd)' will return a NULL pointer) or by dup2()'ing a completely different file object to that very file descriptor, leading to the same situation: a dangling file descriptor pointing to a freed object -- just in this case to a file object of user's choosing. Generally speaking, after the call to fd_install() the file descriptor is live and userland is free to do whatever with it. We cannot rely on it to still refer to our enclave object afterwards. In fact, by abusing userfaultfd() userland can hit the condition without any racing and abuse the error handling in the nitro code as it pleases. To fix the above issues, defer the call to fd_install() until all possible errors are handled. In this case it's just the usercopy, so do it directly in ne_create_vm_ioctl() itself. Signed-off-by: Mathias Krause Signed-off-by: Andra Paraschiv Cc: stable Link: https://lore.kernel.org/r/20210429165941.27020-2-andraprs@amazon.com Signed-off-by: Greg Kroah-Hartman --- drivers/virt/nitro_enclaves/ne_misc_dev.c | 43 +++++++++++------------------- 1 file changed, 17 insertions(+), 26 deletions(-) --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c @@ -1524,7 +1524,8 @@ static const struct file_operations ne_e * enclave file descriptor to be further used for enclave * resources handling e.g. memory regions and CPUs. * @ne_pci_dev : Private data associated with the PCI device. - * @slot_uid: Generated unique slot id associated with an enclave. + * @slot_uid: User pointer to store the generated unique slot id + * associated with an enclave to. * * Context: Process context. This function is called with the ne_pci_dev enclave * mutex held. @@ -1532,7 +1533,7 @@ static const struct file_operations ne_e * * Enclave fd on success. * * Negative return value on failure. */ -static int ne_create_vm_ioctl(struct ne_pci_dev *ne_pci_dev, u64 *slot_uid) +static int ne_create_vm_ioctl(struct ne_pci_dev *ne_pci_dev, u64 __user *slot_uid) { struct ne_pci_dev_cmd_reply cmd_reply = {}; int enclave_fd = -1; @@ -1634,7 +1635,18 @@ static int ne_create_vm_ioctl(struct ne_ list_add(&ne_enclave->enclave_list_entry, &ne_pci_dev->enclaves_list); - *slot_uid = ne_enclave->slot_uid; + if (copy_to_user(slot_uid, &ne_enclave->slot_uid, sizeof(ne_enclave->slot_uid))) { + /* + * As we're holding the only reference to 'enclave_file', fput() + * will call ne_enclave_release() which will do a proper cleanup + * of all so far allocated resources, leaving only the unused fd + * for us to free. + */ + fput(enclave_file); + put_unused_fd(enclave_fd); + + return -EFAULT; + } fd_install(enclave_fd, enclave_file); @@ -1671,34 +1683,13 @@ static long ne_ioctl(struct file *file, switch (cmd) { case NE_CREATE_VM: { int enclave_fd = -1; - struct file *enclave_file = NULL; struct ne_pci_dev *ne_pci_dev = ne_devs.ne_pci_dev; - int rc = -EINVAL; - u64 slot_uid = 0; + u64 __user *slot_uid = (void __user *)arg; mutex_lock(&ne_pci_dev->enclaves_list_mutex); - - enclave_fd = ne_create_vm_ioctl(ne_pci_dev, &slot_uid); - if (enclave_fd < 0) { - rc = enclave_fd; - - mutex_unlock(&ne_pci_dev->enclaves_list_mutex); - - return rc; - } - + enclave_fd = ne_create_vm_ioctl(ne_pci_dev, slot_uid); mutex_unlock(&ne_pci_dev->enclaves_list_mutex); - if (copy_to_user((void __user *)arg, &slot_uid, sizeof(slot_uid))) { - enclave_file = fget(enclave_fd); - /* Decrement file refs to have release() called. */ - fput(enclave_file); - fput(enclave_file); - put_unused_fd(enclave_fd); - - return -EFAULT; - } - return enclave_fd; }