From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C315C43600 for ; Fri, 14 May 2021 09:25:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D8786613FB for ; Fri, 14 May 2021 09:25:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232409AbhENJ1B (ORCPT ); Fri, 14 May 2021 05:27:01 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:46021 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231142AbhENJ1A (ORCPT ); Fri, 14 May 2021 05:27:00 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620984349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=S5lFNU5TQKD03U47KGRUq0qxXj401VvzZwf8ycDk75sZ8elD6zmYfr8ormTPVmnMFlB//f k7DIspm8Yj8j769M+pu58fgnypnwOYb1A7IAdw2hBPD/n1GZwGdpwstsH11YZVHvVvpX7A tkxkRTd+2YudBVAMGoeBZyeF6WdzMDE= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-246-YUI--F8FNWa8GFb8V3vq0g-1; Fri, 14 May 2021 05:25:47 -0400 X-MC-Unique: YUI--F8FNWa8GFb8V3vq0g-1 Received: by mail-ed1-f71.google.com with SMTP id z12-20020aa7d40c0000b0290388179cc8bfso15995007edq.21 for ; Fri, 14 May 2021 02:25:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:organization:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=le9azDJkKzeg7J2uFxsv5ciJsO+EWFy6qHw75ZlVDMOdpaCb3iDzNfK6aUDVofeltU 5WdCmewPnsBC77iLLyYSusjCBzEndSJUQYYPk3QhRobDu1tADuAmAqZp0jHNUb1OCEhV 1WCXSRUDh7oI15BsYGXZJspNV/886ff9slfF3AlfgUEtW0nMBHF5aAxlwyVCZXWC65LN nnYRAJwGZRh+EFI10rHBCZI1xItyEfUxkz9oStYxfNGxwXymkiN4yLAzcUhMOERvOTSq WpnRQPXh+VS/UHvhlctFFByQTRFC9znAox2+PdRnQoZvU3sLHy3famreyfnlbAPx2XxR 2sIA== X-Gm-Message-State: AOAM532SLpcMPGTPbRjuCAJMKeG7vf1S4DzNhlVko7zoAxjKqCjN0f/P GEOtkCQjQBEFM77ECTcsTv4El79r6SGFgoSaWhBbH8lXazVj2dfqwqPklCYD3k6mJcp+0X2nXfP sx9qf0vV4mszjHZSPgVg8a5AX X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117142edj.5.1620984346390; Fri, 14 May 2021 02:25:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwffpojWIdDjWYW/MMXX/wRjb+C923CYpz5CDGlKRq9NSx2v+gQE16nA0IMA6MthtTXx38cuQ== X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117114edj.5.1620984346105; Fri, 14 May 2021 02:25:46 -0700 (PDT) Received: from [192.168.3.132] (p5b0c6501.dip0.t-ipconnect.de. [91.12.101.1]) by smtp.gmail.com with ESMTPSA id gt12sm3267244ejb.60.2021.05.14.02.25.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 May 2021 02:25:45 -0700 (PDT) To: Mike Rapoport , Andrew Morton Cc: Alexander Viro , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Catalin Marinas , Christopher Lameter , Dan Williams , Dave Hansen , Elena Reshetova , "H. Peter Anvin" , Hagen Paul Pfeifer , Ingo Molnar , James Bottomley , Kees Cook , "Kirill A. Shutemov" , Matthew Wilcox , Matthew Garrett , Mark Rutland , Michal Hocko , Mike Rapoport , Michael Kerrisk , Palmer Dabbelt , Palmer Dabbelt , Paul Walmsley , Peter Zijlstra , "Rafael J. Wysocki" , Rick Edgecombe , Roman Gushchin , Shakeel Butt , Shuah Khan , Thomas Gleixner , Tycho Andersen , Will Deacon , Yury Norov , linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-riscv@lists.infradead.org, x86@kernel.org References: <20210513184734.29317-1-rppt@kernel.org> <20210513184734.29317-6-rppt@kernel.org> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v19 5/8] mm: introduce memfd_secret system call to create "secret" memory areas Message-ID: Date: Fri, 14 May 2021 11:25:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210513184734.29317-6-rppt@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > #ifdef CONFIG_IA64 > # include > @@ -64,6 +65,9 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size) > #ifdef CONFIG_STRICT_DEVMEM > static inline int page_is_allowed(unsigned long pfn) > { > + if (pfn_valid(pfn) && page_is_secretmem(pfn_to_page(pfn))) > + return 0; > + 1. The memmap might be garbage. You should use pfn_to_online_page() instead. page = pfn_to_online_page(pfn); if (page && page_is_secretmem(page)) return 0; 2. What about !CONFIG_STRICT_DEVMEM? 3. Someone could map physical memory before a secretmem page gets allocated and read the content after it got allocated and gets used. If someone would gain root privileges and would wait for the target application to (re)start, that could be problematic. I do wonder if enforcing CONFIG_STRICT_DEVMEM would be cleaner. devmem_is_allowed() should disallow access to any system ram, and thereby, any possible secretmem pages, avoiding this check completely. [...] > > diff --git a/mm/secretmem.c b/mm/secretmem.c > new file mode 100644 > index 000000000000..1ae50089adf1 > --- /dev/null > +++ b/mm/secretmem.c > @@ -0,0 +1,239 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright IBM Corporation, 2021 > + * > + * Author: Mike Rapoport > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > + > +#include "internal.h" > + > +#undef pr_fmt > +#define pr_fmt(fmt) "secretmem: " fmt > + > +/* > + * Define mode and flag masks to allow validation of the system call > + * parameters. > + */ > +#define SECRETMEM_MODE_MASK (0x0) > +#define SECRETMEM_FLAGS_MASK SECRETMEM_MODE_MASK > + > +static bool secretmem_enable __ro_after_init; > +module_param_named(enable, secretmem_enable, bool, 0400); > +MODULE_PARM_DESC(secretmem_enable, > + "Enable secretmem and memfd_secret(2) system call"); > + > +static vm_fault_t secretmem_fault(struct vm_fault *vmf) > +{ > + struct address_space *mapping = vmf->vma->vm_file->f_mapping; > + struct inode *inode = file_inode(vmf->vma->vm_file); > + pgoff_t offset = vmf->pgoff; > + gfp_t gfp = vmf->gfp_mask; > + unsigned long addr; > + struct page *page; > + int err; > + > + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) > + return vmf_error(-EINVAL); > + > +retry: > + page = find_lock_page(mapping, offset); > + if (!page) { > + page = alloc_page(gfp | __GFP_ZERO); We'll end up here with gfp == GFP_HIGHUSER (via the mapping below), correct? > + if (!page) > + return VM_FAULT_OOM; > + > + err = set_direct_map_invalid_noflush(page, 1); > + if (err) { > + put_page(page); > + return vmf_error(err); Would we want to translate that to a proper VM_FAULT_..., which would most probably be VM_FAULT_OOM when we fail to allocate a pagetable? > + } > + > + __SetPageUptodate(page); > + err = add_to_page_cache_lru(page, mapping, offset, gfp); > + if (unlikely(err)) { > + put_page(page); > + /* > + * If a split of large page was required, it > + * already happened when we marked the page invalid > + * which guarantees that this call won't fail > + */ > + set_direct_map_default_noflush(page, 1); > + if (err == -EEXIST) > + goto retry; > + > + return vmf_error(err); > + } > + > + addr = (unsigned long)page_address(page); > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE); Hmm, to me it feels like something like that belongs into the set_direct_map_invalid_*() calls? Otherwise it's just very easy to mess up ... I'm certainly not a filesystem guy. Nothing else jumped at me. To me, the overall approach makes sense and I consider it an improved mlock() mechanism for storing secrets, although I'd love to have some more information in the log regarding access via root, namely that there are still fancy ways to read secretmem memory once root via 1. warm reboot attacks especially in VMs (e.g., modifying the cmdline) 2. kexec-style reboot attacks (e.g., modifying the cmdline) 3. kdump attacks 4. kdb most probably 5. "letting the process read the memory for us" via Kees if that still applies 6. ... most probably something else Just to make people aware that there are still some things to be sorted out when we fully want to protect against privilege escalations. (maybe this information is buried in the cover letter already, where it usually gets lost) -- Thanks, David / dhildenb From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 22B34C433B4 for ; Fri, 14 May 2021 09:26:13 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 7710E613FB for ; Fri, 14 May 2021 09:26:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7710E613FB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:Subject: From:References:Cc:To:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=FBn9Dcmtygx+dL1BrH51jPB2mmZbbK4bdcDpqTpoZEQ=; b=BLyeggY5GpGXvH/LRMGgeeVEr I9y80xVVk+cyss27ZFdn3BtHPrcGeU8rjZnY34CYnyZP2LPuDrjQgyDwLZnDQGwVgQTFIdLh1iA8d 27wmsee9Dr9hW4EQt+AIlIJozbsUJaFV1LUG+JcaTAlQZcFA1W03Xvb93Sijhgl9rrWJeoFby8waw PPhPV6u2q383JIv5Be8cisKiu1tCWswwDOVvdn5uUWftP71Cuh5ghXLEtt12p4EH66OMg6pj4gLWW h3yjcqZn9Gqq+qFKDlHEzFHLjgO6oWLJav9LAGVLc4nrxY2nUk9WH9e806AzEq+/ayUE2uhVvl8L6 3IRWsnr4g==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lhU4y-007cjH-By; Fri, 14 May 2021 09:25:56 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lhU4v-007cij-Vj for linux-riscv@desiato.infradead.org; Fri, 14 May 2021 09:25:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:Subject:From:References :Cc:To:Sender:Reply-To:Content-ID:Content-Description; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=mWDjBydvgTLX95CpCU+XnDUMb5 KQuVqDMU6iZ3PHda5SCyErwloQ4e4aH0DSOchHIDXd3IGsUX0pGhIM96uzQ5OZGOUJtb9XOR8D5ko tLEuT1QLDHcgleX+hfocFHpvRM6u/SSXDZ/d1lniZtNcwGLB3wb3gOq/OQM4zYxi/DfirA8Esi+ru /5Vkk2rfqh0pCCnUH2pmkfnzpHGiX4hmBIlhVzL6VlHumbqtLAxjNi22hqMkZh99d4GJVe2ASaG6t ReD68WtcjXYpE2gtd8yU/GyJ/1f9FylSlNuL3Qv7VBXnOpGa7nP6umYxy/nHDhA1/aY0WCcXTvDX0 abXvSixg==; Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lhU4s-00Br8c-F0 for linux-riscv@lists.infradead.org; Fri, 14 May 2021 09:25:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620984349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=S5lFNU5TQKD03U47KGRUq0qxXj401VvzZwf8ycDk75sZ8elD6zmYfr8ormTPVmnMFlB//f k7DIspm8Yj8j769M+pu58fgnypnwOYb1A7IAdw2hBPD/n1GZwGdpwstsH11YZVHvVvpX7A tkxkRTd+2YudBVAMGoeBZyeF6WdzMDE= Received: from mail-ed1-f70.google.com (mail-ed1-f70.google.com [209.85.208.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-569-nJfsKbSJNq6-yeenWAWx1A-1; Fri, 14 May 2021 05:25:47 -0400 X-MC-Unique: nJfsKbSJNq6-yeenWAWx1A-1 Received: by mail-ed1-f70.google.com with SMTP id o12-20020aa7d3cc0000b02903891695ceabso16081787edr.3 for ; Fri, 14 May 2021 02:25:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:organization:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=WBE2wiyq69CGD/EoQ/KDioahhg4k8rf2CnMtqNtn1btZfHdXrq7MdYXK13r0rtkb9t SwVJVQt/N2P6rwtUFZ9eQC/4V5JzkdD5wLp8GY8Y0NbiSgSRLLjgC3P0SuBoxXYBaJEw 9TWJmcTmBGjNcbHM5r9OUosr+LLNURjRO/tgbBrNYPcUTtNcCviBk1GuLKXGRLeOo97K Zfk81s68kDLJu70z2KMk7tAIH0g2t6ShxYawC4cUFyutPSC8s7XF/4H01vPybxAcPryn x3GzCQWbY2qxkScGYbynRA8vlpB96kUTYNdTH10T3y5F1YeJsj1ctNzbA7qP9EYVic5v cI3w== X-Gm-Message-State: AOAM532qU/Du+CNy4UvqcUJFNebqBUb3NaNKtcgWf6g7EyzfpBEjxm7W 3DnSm28Ht/IeL3AzSZ7Qvr1oHFI0bOF6lQOU4bic9viyE36Eii5bIjFIRK/yU3w3Nsxm1Ywi5cX J4NUB6XaZIPYHBIIHw1lch7dEnKhe X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117162edj.5.1620984346407; Fri, 14 May 2021 02:25:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwffpojWIdDjWYW/MMXX/wRjb+C923CYpz5CDGlKRq9NSx2v+gQE16nA0IMA6MthtTXx38cuQ== X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117114edj.5.1620984346105; Fri, 14 May 2021 02:25:46 -0700 (PDT) Received: from [192.168.3.132] (p5b0c6501.dip0.t-ipconnect.de. [91.12.101.1]) by smtp.gmail.com with ESMTPSA id gt12sm3267244ejb.60.2021.05.14.02.25.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 May 2021 02:25:45 -0700 (PDT) To: Mike Rapoport , Andrew Morton Cc: Alexander Viro , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Catalin Marinas , Christopher Lameter , Dan Williams , Dave Hansen , Elena Reshetova , "H. Peter Anvin" , Hagen Paul Pfeifer , Ingo Molnar , James Bottomley , Kees Cook , "Kirill A. Shutemov" , Matthew Wilcox , Matthew Garrett , Mark Rutland , Michal Hocko , Mike Rapoport , Michael Kerrisk , Palmer Dabbelt , Palmer Dabbelt , Paul Walmsley , Peter Zijlstra , "Rafael J. Wysocki" , Rick Edgecombe , Roman Gushchin , Shakeel Butt , Shuah Khan , Thomas Gleixner , Tycho Andersen , Will Deacon , Yury Norov , linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-riscv@lists.infradead.org, x86@kernel.org References: <20210513184734.29317-1-rppt@kernel.org> <20210513184734.29317-6-rppt@kernel.org> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v19 5/8] mm: introduce memfd_secret system call to create "secret" memory areas Message-ID: Date: Fri, 14 May 2021 11:25:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210513184734.29317-6-rppt@kernel.org> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210514_022550_609975_99E9181A X-CRM114-Status: GOOD ( 31.17 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org > #ifdef CONFIG_IA64 > # include > @@ -64,6 +65,9 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size) > #ifdef CONFIG_STRICT_DEVMEM > static inline int page_is_allowed(unsigned long pfn) > { > + if (pfn_valid(pfn) && page_is_secretmem(pfn_to_page(pfn))) > + return 0; > + 1. The memmap might be garbage. You should use pfn_to_online_page() instead. page = pfn_to_online_page(pfn); if (page && page_is_secretmem(page)) return 0; 2. What about !CONFIG_STRICT_DEVMEM? 3. Someone could map physical memory before a secretmem page gets allocated and read the content after it got allocated and gets used. If someone would gain root privileges and would wait for the target application to (re)start, that could be problematic. I do wonder if enforcing CONFIG_STRICT_DEVMEM would be cleaner. devmem_is_allowed() should disallow access to any system ram, and thereby, any possible secretmem pages, avoiding this check completely. [...] > > diff --git a/mm/secretmem.c b/mm/secretmem.c > new file mode 100644 > index 000000000000..1ae50089adf1 > --- /dev/null > +++ b/mm/secretmem.c > @@ -0,0 +1,239 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright IBM Corporation, 2021 > + * > + * Author: Mike Rapoport > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > + > +#include "internal.h" > + > +#undef pr_fmt > +#define pr_fmt(fmt) "secretmem: " fmt > + > +/* > + * Define mode and flag masks to allow validation of the system call > + * parameters. > + */ > +#define SECRETMEM_MODE_MASK (0x0) > +#define SECRETMEM_FLAGS_MASK SECRETMEM_MODE_MASK > + > +static bool secretmem_enable __ro_after_init; > +module_param_named(enable, secretmem_enable, bool, 0400); > +MODULE_PARM_DESC(secretmem_enable, > + "Enable secretmem and memfd_secret(2) system call"); > + > +static vm_fault_t secretmem_fault(struct vm_fault *vmf) > +{ > + struct address_space *mapping = vmf->vma->vm_file->f_mapping; > + struct inode *inode = file_inode(vmf->vma->vm_file); > + pgoff_t offset = vmf->pgoff; > + gfp_t gfp = vmf->gfp_mask; > + unsigned long addr; > + struct page *page; > + int err; > + > + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) > + return vmf_error(-EINVAL); > + > +retry: > + page = find_lock_page(mapping, offset); > + if (!page) { > + page = alloc_page(gfp | __GFP_ZERO); We'll end up here with gfp == GFP_HIGHUSER (via the mapping below), correct? > + if (!page) > + return VM_FAULT_OOM; > + > + err = set_direct_map_invalid_noflush(page, 1); > + if (err) { > + put_page(page); > + return vmf_error(err); Would we want to translate that to a proper VM_FAULT_..., which would most probably be VM_FAULT_OOM when we fail to allocate a pagetable? > + } > + > + __SetPageUptodate(page); > + err = add_to_page_cache_lru(page, mapping, offset, gfp); > + if (unlikely(err)) { > + put_page(page); > + /* > + * If a split of large page was required, it > + * already happened when we marked the page invalid > + * which guarantees that this call won't fail > + */ > + set_direct_map_default_noflush(page, 1); > + if (err == -EEXIST) > + goto retry; > + > + return vmf_error(err); > + } > + > + addr = (unsigned long)page_address(page); > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE); Hmm, to me it feels like something like that belongs into the set_direct_map_invalid_*() calls? Otherwise it's just very easy to mess up ... I'm certainly not a filesystem guy. Nothing else jumped at me. To me, the overall approach makes sense and I consider it an improved mlock() mechanism for storing secrets, although I'd love to have some more information in the log regarding access via root, namely that there are still fancy ways to read secretmem memory once root via 1. warm reboot attacks especially in VMs (e.g., modifying the cmdline) 2. kexec-style reboot attacks (e.g., modifying the cmdline) 3. kdump attacks 4. kdb most probably 5. "letting the process read the memory for us" via Kees if that still applies 6. ... most probably something else Just to make people aware that there are still some things to be sorted out when we fully want to protect against privilege escalations. (maybe this information is buried in the cover letter already, where it usually gets lost) -- Thanks, David / dhildenb _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7B2E8C433ED for ; Fri, 14 May 2021 09:25:56 +0000 (UTC) Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D20A361406 for ; Fri, 14 May 2021 09:25:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org D20A361406 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-nvdimm-bounces@lists.01.org Received: from ml01.vlan13.01.org (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 6EDDB100EAB07; Fri, 14 May 2021 02:25:53 -0700 (PDT) Received-SPF: Pass (mailfrom) identity=mailfrom; client-ip=216.205.24.124; helo=us-smtp-delivery-124.mimecast.com; envelope-from=david@redhat.com; receiver= Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7B02F100EAB02 for ; Fri, 14 May 2021 02:25:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620984349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=S5lFNU5TQKD03U47KGRUq0qxXj401VvzZwf8ycDk75sZ8elD6zmYfr8ormTPVmnMFlB//f k7DIspm8Yj8j769M+pu58fgnypnwOYb1A7IAdw2hBPD/n1GZwGdpwstsH11YZVHvVvpX7A tkxkRTd+2YudBVAMGoeBZyeF6WdzMDE= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-569-4y7rghqxPMeVdtlffZ-PSA-1; Fri, 14 May 2021 05:25:47 -0400 X-MC-Unique: 4y7rghqxPMeVdtlffZ-PSA-1 Received: by mail-ed1-f72.google.com with SMTP id q18-20020a50cc920000b029038cf491864cso745212edi.14 for ; Fri, 14 May 2021 02:25:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:organization:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=GgJrFm2+20GfI/X+d6KA0f9PPJ7qIYXz1A387M81YPxckAeiIesVZ3gCAH1kn5zMTg 0kNTCHAJAOqq/NZ1puxFYyOBWMd7cD14Pt5eda1nmGaUONdFF1pQ79duAiG5DVEexmXN L16WOFPNfg/UqHPjaaan26nRXkT3gBQ+m3O71/pa82XXpLioQ0bzn5T5FocGZiwtoh7j VApC9VO8tQ4PvnpneUrIlA+VlEEWxYM5aq9ijMlg1Ng1JXNxeUqDzQLhSsF9H59fcXCE rPaPejyJu3wu/Ea1Y5ZYuZ0zhyTR4MJGV090Rvul6YXFFPdrhtkTRQcL1h+29EZa9L2K 15bw== X-Gm-Message-State: AOAM531+QvSaGyYebrdAl8T4+H97Fr08zBTdd5B07LK3WSU3ysE31F3k KecpQceTqwoEyat/kQyCPrMn2DQLnYnCp6NTkyhcgAbVarYlwShO3DkH6lo/f0pa3/FMakSACSI VATWn9Urh8HrUN3g9gdqe X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117163edj.5.1620984346410; Fri, 14 May 2021 02:25:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwffpojWIdDjWYW/MMXX/wRjb+C923CYpz5CDGlKRq9NSx2v+gQE16nA0IMA6MthtTXx38cuQ== X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117114edj.5.1620984346105; Fri, 14 May 2021 02:25:46 -0700 (PDT) Received: from [192.168.3.132] (p5b0c6501.dip0.t-ipconnect.de. [91.12.101.1]) by smtp.gmail.com with ESMTPSA id gt12sm3267244ejb.60.2021.05.14.02.25.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 May 2021 02:25:45 -0700 (PDT) To: Mike Rapoport , Andrew Morton References: <20210513184734.29317-1-rppt@kernel.org> <20210513184734.29317-6-rppt@kernel.org> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v19 5/8] mm: introduce memfd_secret system call to create "secret" memory areas Message-ID: Date: Fri, 14 May 2021 11:25:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210513184734.29317-6-rppt@kernel.org> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Message-ID-Hash: DA2RBDDL2GXALVY5DYYT4VEP63Q36H4G X-Message-ID-Hash: DA2RBDDL2GXALVY5DYYT4VEP63Q36H4G X-MailFrom: david@redhat.com X-Mailman-Rule-Hits: nonmember-moderation X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation CC: Alexander Viro , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Catalin Marinas , Christopher Lameter , Dave Hansen , Elena Reshetova , "H. Peter Anvin" , Hagen Paul Pfeifer , Ingo Molnar , James Bottomley , Kees Cook , "Kirill A. Shutemov" , Matthew Wilcox , Matthew Garrett , Mark Rutland , Michal Hocko , Mike Rapoport , Michael Kerrisk , Palmer Dabbelt , Palmer Dabbelt , Paul Walmsley , Peter Zijlstra , "Rafael J. Wysocki" , Rick Edgecombe , Roman Gushchin , Shakeel Butt , Shuah Khan , Thomas Gleixner , Tycho Andersen , Will Deacon , Yury Norov , linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-riscv@lists.infradead.org, x86@kernel.org X-Mailman-Version: 3.1.1 Precedence: list List-Id: "Linux-nvdimm developer list." Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii"; format="flowed" Content-Transfer-Encoding: 7bit > #ifdef CONFIG_IA64 > # include > @@ -64,6 +65,9 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size) > #ifdef CONFIG_STRICT_DEVMEM > static inline int page_is_allowed(unsigned long pfn) > { > + if (pfn_valid(pfn) && page_is_secretmem(pfn_to_page(pfn))) > + return 0; > + 1. The memmap might be garbage. You should use pfn_to_online_page() instead. page = pfn_to_online_page(pfn); if (page && page_is_secretmem(page)) return 0; 2. What about !CONFIG_STRICT_DEVMEM? 3. Someone could map physical memory before a secretmem page gets allocated and read the content after it got allocated and gets used. If someone would gain root privileges and would wait for the target application to (re)start, that could be problematic. I do wonder if enforcing CONFIG_STRICT_DEVMEM would be cleaner. devmem_is_allowed() should disallow access to any system ram, and thereby, any possible secretmem pages, avoiding this check completely. [...] > > diff --git a/mm/secretmem.c b/mm/secretmem.c > new file mode 100644 > index 000000000000..1ae50089adf1 > --- /dev/null > +++ b/mm/secretmem.c > @@ -0,0 +1,239 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright IBM Corporation, 2021 > + * > + * Author: Mike Rapoport > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > + > +#include "internal.h" > + > +#undef pr_fmt > +#define pr_fmt(fmt) "secretmem: " fmt > + > +/* > + * Define mode and flag masks to allow validation of the system call > + * parameters. > + */ > +#define SECRETMEM_MODE_MASK (0x0) > +#define SECRETMEM_FLAGS_MASK SECRETMEM_MODE_MASK > + > +static bool secretmem_enable __ro_after_init; > +module_param_named(enable, secretmem_enable, bool, 0400); > +MODULE_PARM_DESC(secretmem_enable, > + "Enable secretmem and memfd_secret(2) system call"); > + > +static vm_fault_t secretmem_fault(struct vm_fault *vmf) > +{ > + struct address_space *mapping = vmf->vma->vm_file->f_mapping; > + struct inode *inode = file_inode(vmf->vma->vm_file); > + pgoff_t offset = vmf->pgoff; > + gfp_t gfp = vmf->gfp_mask; > + unsigned long addr; > + struct page *page; > + int err; > + > + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) > + return vmf_error(-EINVAL); > + > +retry: > + page = find_lock_page(mapping, offset); > + if (!page) { > + page = alloc_page(gfp | __GFP_ZERO); We'll end up here with gfp == GFP_HIGHUSER (via the mapping below), correct? > + if (!page) > + return VM_FAULT_OOM; > + > + err = set_direct_map_invalid_noflush(page, 1); > + if (err) { > + put_page(page); > + return vmf_error(err); Would we want to translate that to a proper VM_FAULT_..., which would most probably be VM_FAULT_OOM when we fail to allocate a pagetable? > + } > + > + __SetPageUptodate(page); > + err = add_to_page_cache_lru(page, mapping, offset, gfp); > + if (unlikely(err)) { > + put_page(page); > + /* > + * If a split of large page was required, it > + * already happened when we marked the page invalid > + * which guarantees that this call won't fail > + */ > + set_direct_map_default_noflush(page, 1); > + if (err == -EEXIST) > + goto retry; > + > + return vmf_error(err); > + } > + > + addr = (unsigned long)page_address(page); > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE); Hmm, to me it feels like something like that belongs into the set_direct_map_invalid_*() calls? Otherwise it's just very easy to mess up ... I'm certainly not a filesystem guy. Nothing else jumped at me. To me, the overall approach makes sense and I consider it an improved mlock() mechanism for storing secrets, although I'd love to have some more information in the log regarding access via root, namely that there are still fancy ways to read secretmem memory once root via 1. warm reboot attacks especially in VMs (e.g., modifying the cmdline) 2. kexec-style reboot attacks (e.g., modifying the cmdline) 3. kdump attacks 4. kdb most probably 5. "letting the process read the memory for us" via Kees if that still applies 6. ... most probably something else Just to make people aware that there are still some things to be sorted out when we fully want to protect against privilege escalations. (maybe this information is buried in the cover letter already, where it usually gets lost) -- Thanks, David / dhildenb _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A3CF1C433ED for ; Fri, 14 May 2021 09:27:58 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 298DD613F6 for ; Fri, 14 May 2021 09:27:58 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 298DD613F6 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:Subject: From:References:Cc:To:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=6Jei+i3aeaU79RjliOCWdcA7ZKS073vwVmzcomnSyJ8=; b=b1p5dfna5efG1aY/tQyPD20Wf u0sWxx849THqdm8jq0CinDJNVgHgFIKw/F5SgWIQWxhG2ULRpQuA1Fh5wscKyttKIQoZI1Ummt96H 8/oRDfpDkvPv4tVcY/sk1ozW6Z1kaANE8DF1FkO7x0SP4eQF8hjYNk/Z7qFlDreopWIEJEu3Thfns zA4XALQ5TqZTqinQzFBTX3qXAQSOcllYBbNEWbMOaw3VEtdw82T2Sm7f8Fav5hnhZSuARNseNM53n 3rTu3wMR9dogu4VnL2h+wr3ZklU7Kn44cSVIUu+2NlS8a1JDHm8qYU6xl+O8BbP1HqQXGhdE8CQFb PPcfqbv+w==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lhU51-007cjh-Ph; Fri, 14 May 2021 09:26:00 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lhU4v-007cik-Vj for linux-arm-kernel@desiato.infradead.org; Fri, 14 May 2021 09:25:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Content-Transfer-Encoding: Content-Type:In-Reply-To:MIME-Version:Date:Message-ID:Subject:From:References :Cc:To:Sender:Reply-To:Content-ID:Content-Description; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=mWDjBydvgTLX95CpCU+XnDUMb5 KQuVqDMU6iZ3PHda5SCyErwloQ4e4aH0DSOchHIDXd3IGsUX0pGhIM96uzQ5OZGOUJtb9XOR8D5ko tLEuT1QLDHcgleX+hfocFHpvRM6u/SSXDZ/d1lniZtNcwGLB3wb3gOq/OQM4zYxi/DfirA8Esi+ru /5Vkk2rfqh0pCCnUH2pmkfnzpHGiX4hmBIlhVzL6VlHumbqtLAxjNi22hqMkZh99d4GJVe2ASaG6t ReD68WtcjXYpE2gtd8yU/GyJ/1f9FylSlNuL3Qv7VBXnOpGa7nP6umYxy/nHDhA1/aY0WCcXTvDX0 abXvSixg==; Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lhU4s-00Br8Z-Fp for linux-arm-kernel@lists.infradead.org; Fri, 14 May 2021 09:25:52 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1620984349; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=S5lFNU5TQKD03U47KGRUq0qxXj401VvzZwf8ycDk75sZ8elD6zmYfr8ormTPVmnMFlB//f k7DIspm8Yj8j769M+pu58fgnypnwOYb1A7IAdw2hBPD/n1GZwGdpwstsH11YZVHvVvpX7A tkxkRTd+2YudBVAMGoeBZyeF6WdzMDE= Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-378-Ah6iSP-DPq28ZuI4Fq7IPQ-1; Fri, 14 May 2021 05:25:47 -0400 X-MC-Unique: Ah6iSP-DPq28ZuI4Fq7IPQ-1 Received: by mail-ed1-f69.google.com with SMTP id n6-20020a0564020606b029038cdc241890so2563337edv.20 for ; Fri, 14 May 2021 02:25:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:to:cc:references:from:organization:subject :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=Fx334L5vVgVDfx7L2kVeRZ1gFR/dFDyujA5YMe7z6io=; b=MzsD6X7UvI3SQgdRtnjFT6lSZbDCZmItoxG6pj3fOkPLSzKieANRN4Vqn8rBHV5dwH 8E0A6fnwTlkXDmXxWoaDFbf3n5OfQSTKDV5U4Hat9RnPJqTl1t1LrrwjGZ1jSOIyuULW hJpVeFcvdZ/RrlDO1J5zoiIml0jfYhq5fgkmc8qzxh5+L7LZFNlyc36ea8EhnUEKcbS8 38XumcX1zR7nVnucdIp4WzZa/2ANTCNy01Qvd3G8TuWRYO+J8m4Q3GRr6/CpkLEykTF6 paw+uYQsEmrF1Kju5917di+s57mptHDfV/2B9T3I6L6y3NzgpKIy9VK47PGlvfIwqtYY l5og== X-Gm-Message-State: AOAM5321t6WWDwzx4A1fdnOqOcz6FhuM6iXuadpvwQF5hoaL/JvePAtm iL4H371TmQv3RGSw4IOId//IgM87LMx81UeSg9gySC7xx0duSJFLi8pR7JbJ3uu/PKzH5h5W+Pq srTS1BN17UnHXEG2dXlathFMYalRjS/QGQPE= X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117185edj.5.1620984346510; Fri, 14 May 2021 02:25:46 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwffpojWIdDjWYW/MMXX/wRjb+C923CYpz5CDGlKRq9NSx2v+gQE16nA0IMA6MthtTXx38cuQ== X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117114edj.5.1620984346105; Fri, 14 May 2021 02:25:46 -0700 (PDT) Received: from [192.168.3.132] (p5b0c6501.dip0.t-ipconnect.de. [91.12.101.1]) by smtp.gmail.com with ESMTPSA id gt12sm3267244ejb.60.2021.05.14.02.25.44 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 14 May 2021 02:25:45 -0700 (PDT) To: Mike Rapoport , Andrew Morton Cc: Alexander Viro , Andy Lutomirski , Arnd Bergmann , Borislav Petkov , Catalin Marinas , Christopher Lameter , Dan Williams , Dave Hansen , Elena Reshetova , "H. Peter Anvin" , Hagen Paul Pfeifer , Ingo Molnar , James Bottomley , Kees Cook , "Kirill A. Shutemov" , Matthew Wilcox , Matthew Garrett , Mark Rutland , Michal Hocko , Mike Rapoport , Michael Kerrisk , Palmer Dabbelt , Palmer Dabbelt , Paul Walmsley , Peter Zijlstra , "Rafael J. Wysocki" , Rick Edgecombe , Roman Gushchin , Shakeel Butt , Shuah Khan , Thomas Gleixner , Tycho Andersen , Will Deacon , Yury Norov , linux-api@vger.kernel.org, linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-riscv@lists.infradead.org, x86@kernel.org References: <20210513184734.29317-1-rppt@kernel.org> <20210513184734.29317-6-rppt@kernel.org> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v19 5/8] mm: introduce memfd_secret system call to create "secret" memory areas Message-ID: Date: Fri, 14 May 2021 11:25:43 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210513184734.29317-6-rppt@kernel.org> Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210514_022550_617907_74D65F1D X-CRM114-Status: GOOD ( 32.77 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > #ifdef CONFIG_IA64 > # include > @@ -64,6 +65,9 @@ static inline int valid_mmap_phys_addr_range(unsigned long pfn, size_t size) > #ifdef CONFIG_STRICT_DEVMEM > static inline int page_is_allowed(unsigned long pfn) > { > + if (pfn_valid(pfn) && page_is_secretmem(pfn_to_page(pfn))) > + return 0; > + 1. The memmap might be garbage. You should use pfn_to_online_page() instead. page = pfn_to_online_page(pfn); if (page && page_is_secretmem(page)) return 0; 2. What about !CONFIG_STRICT_DEVMEM? 3. Someone could map physical memory before a secretmem page gets allocated and read the content after it got allocated and gets used. If someone would gain root privileges and would wait for the target application to (re)start, that could be problematic. I do wonder if enforcing CONFIG_STRICT_DEVMEM would be cleaner. devmem_is_allowed() should disallow access to any system ram, and thereby, any possible secretmem pages, avoiding this check completely. [...] > > diff --git a/mm/secretmem.c b/mm/secretmem.c > new file mode 100644 > index 000000000000..1ae50089adf1 > --- /dev/null > +++ b/mm/secretmem.c > @@ -0,0 +1,239 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright IBM Corporation, 2021 > + * > + * Author: Mike Rapoport > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > + > +#include > + > +#include "internal.h" > + > +#undef pr_fmt > +#define pr_fmt(fmt) "secretmem: " fmt > + > +/* > + * Define mode and flag masks to allow validation of the system call > + * parameters. > + */ > +#define SECRETMEM_MODE_MASK (0x0) > +#define SECRETMEM_FLAGS_MASK SECRETMEM_MODE_MASK > + > +static bool secretmem_enable __ro_after_init; > +module_param_named(enable, secretmem_enable, bool, 0400); > +MODULE_PARM_DESC(secretmem_enable, > + "Enable secretmem and memfd_secret(2) system call"); > + > +static vm_fault_t secretmem_fault(struct vm_fault *vmf) > +{ > + struct address_space *mapping = vmf->vma->vm_file->f_mapping; > + struct inode *inode = file_inode(vmf->vma->vm_file); > + pgoff_t offset = vmf->pgoff; > + gfp_t gfp = vmf->gfp_mask; > + unsigned long addr; > + struct page *page; > + int err; > + > + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >= i_size_read(inode)) > + return vmf_error(-EINVAL); > + > +retry: > + page = find_lock_page(mapping, offset); > + if (!page) { > + page = alloc_page(gfp | __GFP_ZERO); We'll end up here with gfp == GFP_HIGHUSER (via the mapping below), correct? > + if (!page) > + return VM_FAULT_OOM; > + > + err = set_direct_map_invalid_noflush(page, 1); > + if (err) { > + put_page(page); > + return vmf_error(err); Would we want to translate that to a proper VM_FAULT_..., which would most probably be VM_FAULT_OOM when we fail to allocate a pagetable? > + } > + > + __SetPageUptodate(page); > + err = add_to_page_cache_lru(page, mapping, offset, gfp); > + if (unlikely(err)) { > + put_page(page); > + /* > + * If a split of large page was required, it > + * already happened when we marked the page invalid > + * which guarantees that this call won't fail > + */ > + set_direct_map_default_noflush(page, 1); > + if (err == -EEXIST) > + goto retry; > + > + return vmf_error(err); > + } > + > + addr = (unsigned long)page_address(page); > + flush_tlb_kernel_range(addr, addr + PAGE_SIZE); Hmm, to me it feels like something like that belongs into the set_direct_map_invalid_*() calls? Otherwise it's just very easy to mess up ... I'm certainly not a filesystem guy. Nothing else jumped at me. To me, the overall approach makes sense and I consider it an improved mlock() mechanism for storing secrets, although I'd love to have some more information in the log regarding access via root, namely that there are still fancy ways to read secretmem memory once root via 1. warm reboot attacks especially in VMs (e.g., modifying the cmdline) 2. kexec-style reboot attacks (e.g., modifying the cmdline) 3. kdump attacks 4. kdb most probably 5. "letting the process read the memory for us" via Kees if that still applies 6. ... most probably something else Just to make people aware that there are still some things to be sorted out when we fully want to protect against privilege escalations. (maybe this information is buried in the cover letter already, where it usually gets lost) -- Thanks, David / dhildenb _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel