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=ham 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 EB98FC43461 for ; Fri, 14 May 2021 09:25:51 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8560561406 for ; Fri, 14 May 2021 09:25:51 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8560561406 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F0AB66B0036; Fri, 14 May 2021 05:25:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id EBA296B006E; Fri, 14 May 2021 05:25:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D0CE56B0070; Fri, 14 May 2021 05:25:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0226.hostedemail.com [216.40.44.226]) by kanga.kvack.org (Postfix) with ESMTP id 9C1486B0036 for ; Fri, 14 May 2021 05:25:50 -0400 (EDT) Received: from smtpin17.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id 2A7A91801D50C for ; Fri, 14 May 2021 09:25:50 +0000 (UTC) X-FDA: 78139304460.17.4105E4B Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf06.hostedemail.com (Postfix) with ESMTP id 8A41CC0007CD for ; Fri, 14 May 2021 09:25:49 +0000 (UTC) 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-246-uXtQEyuQPx2N4oDsPqiC2A-1; Fri, 14 May 2021 05:25:47 -0400 X-MC-Unique: uXtQEyuQPx2N4oDsPqiC2A-1 Received: by mail-ed1-f70.google.com with SMTP id h16-20020a0564020950b029038cbdae8cbaso4587053edz.6 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=MJsWhnSK7Frm2kDt2oQ/RrDI/bpYO9CHsrOrxfPT2Pw6r+PlfQEtpwD78B9YuDtzZB z+eT1fYNr3DXEeVXQaK1CtPiu83kHeuUS57e0c94/z2Tt55CyQnz0uvmpxUbJSVvhZVi ldbH8LKEiuCoWdP8mpZlq5pfwY98zu+Lu4euqV25IchNCJyx0HWOGAa7fkdSYuRH9sWB dygA59jwNTbZhzt/URHzoO4FBklsEWO8TkI+hzh/8V4aob3MfJQZaiVgmO4GlhJl0OPX k0sCFfZ6w4oQM8FOSa4S7fhDL0FK9IurtvL9zNU62OL9kguO1vPHhitXXzhNzoa3mggi kwVQ== X-Gm-Message-State: AOAM531IkCsHqDhA5VJ3O7aVYy7nlJjN1XzbHHzqa893EpAfsVf8eh40 L8aHBDicLF2oAi86SDIdRLSlrZXq+UhvhrYBwIiA9ZaZXiY1VGWjiTu9eDD78kx8oBC90hm3H+G pBLQHg7tYiko= X-Received: by 2002:a50:cd57:: with SMTP id d23mr54117157edj.5.1620984346403; 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> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US X-Rspamd-Queue-Id: 8A41CC0007CD Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=S5lFNU5T; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf06.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=david@redhat.com X-Rspamd-Server: rspam03 X-Stat-Signature: hchmsppnda6n1pmejzu1x88c3793fipx X-HE-Tag: 1620984349-411438 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: > #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() inste= ad. page =3D 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=20 allocated and read the content after it got allocated and gets used. If=20 someone would gain root privileges and would wait for the target=20 application to (re)start, that could be problematic. I do wonder if enforcing CONFIG_STRICT_DEVMEM would be cleaner.=20 devmem_is_allowed() should disallow access to any system ram, and=20 thereby, any possible secretmem pages, avoiding this check completely. [...] > =20 > 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 =3D vmf->vma->vm_file->f_mapping; > + struct inode *inode =3D file_inode(vmf->vma->vm_file); > + pgoff_t offset =3D vmf->pgoff; > + gfp_t gfp =3D vmf->gfp_mask; > + unsigned long addr; > + struct page *page; > + int err; > + > + if (((loff_t)vmf->pgoff << PAGE_SHIFT) >=3D i_size_read(inode)) > + return vmf_error(-EINVAL); > + > +retry: > + page =3D find_lock_page(mapping, offset); > + if (!page) { > + page =3D alloc_page(gfp | __GFP_ZERO); We'll end up here with gfp =3D=3D GFP_HIGHUSER (via the mapping below), c= orrect? > + if (!page) > + return VM_FAULT_OOM; > + > + err =3D 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=20 most probably be VM_FAULT_OOM when we fail to allocate a pagetable? > + } > + > + __SetPageUptodate(page); > + err =3D 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 =3D=3D -EEXIST) > + goto retry; > + > + return vmf_error(err); > + } > + > + addr =3D (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=20 set_direct_map_invalid_*() calls? Otherwise it's just very easy to mess=20 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=20 mlock() mechanism for storing secrets, although I'd love to have some=20 more information in the log regarding access via root, namely that there=20 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=20 out when we fully want to protect against privilege escalations. (maybe this information is buried in the cover letter already, where it=20 usually gets lost) --=20 Thanks, David / dhildenb