linux-kernel - Re: [PATCH v5 04/13] mm/shmem: Restrict MFD_INACCESSIBLE memory against RLIMIT

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <02e18c90-196e-409e-b2ac-822aceea8891@www.fastmail.com>
Date:   Thu, 07 Apr 2022 10:09:55 -0700
From:   "Andy Lutomirski" <luto@...nel.org>
To:     "Sean Christopherson" <seanjc@...gle.com>,
        "Chao Peng" <chao.p.peng@...ux.intel.com>
Cc:     "kvm list" <kvm@...r.kernel.org>,
        "Linux Kernel Mailing List" <linux-kernel@...r.kernel.org>,
        linux-mm@...ck.org, linux-fsdevel@...r.kernel.org,
        "Linux API" <linux-api@...r.kernel.org>, qemu-devel@...gnu.org,
        "Paolo Bonzini" <pbonzini@...hat.com>,
        "Jonathan Corbet" <corbet@....net>,
        "Vitaly Kuznetsov" <vkuznets@...hat.com>,
        "Wanpeng Li" <wanpengli@...cent.com>,
        "Jim Mattson" <jmattson@...gle.com>,
        "Joerg Roedel" <joro@...tes.org>,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Ingo Molnar" <mingo@...hat.com>, "Borislav Petkov" <bp@...en8.de>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "Hugh Dickins" <hughd@...gle.com>,
        "Jeff Layton" <jlayton@...nel.org>,
        "J . Bruce Fields" <bfields@...ldses.org>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Mike Rapoport" <rppt@...nel.org>,
        "Steven Price" <steven.price@....com>,
        "Maciej S . Szmigiero" <mail@...iej.szmigiero.name>,
        "Vlastimil Babka" <vbabka@...e.cz>,
        "Vishal Annapurve" <vannapurve@...gle.com>,
        "Yu Zhang" <yu.c.zhang@...ux.intel.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        "Nakajima, Jun" <jun.nakajima@...el.com>,
        "Dave Hansen" <dave.hansen@...el.com>,
        "Andi Kleen" <ak@...ux.intel.com>,
        "David Hildenbrand" <david@...hat.com>
Subject: Re: [PATCH v5 04/13] mm/shmem: Restrict MFD_INACCESSIBLE memory against
 RLIMIT_MEMLOCK



On Thu, Apr 7, 2022, at 9:05 AM, Sean Christopherson wrote:
> On Thu, Mar 10, 2022, Chao Peng wrote:
>> Since page migration / swapping is not supported yet, MFD_INACCESSIBLE
>> memory behave like longterm pinned pages and thus should be accounted to
>> mm->pinned_vm and be restricted by RLIMIT_MEMLOCK.
>> 
>> Signed-off-by: Chao Peng <chao.p.peng@...ux.intel.com>
>> ---
>>  mm/shmem.c | 25 ++++++++++++++++++++++++-
>>  1 file changed, 24 insertions(+), 1 deletion(-)
>> 
>> diff --git a/mm/shmem.c b/mm/shmem.c
>> index 7b43e274c9a2..ae46fb96494b 100644
>> --- a/mm/shmem.c
>> +++ b/mm/shmem.c
>> @@ -915,14 +915,17 @@ static void notify_fallocate(struct inode *inode, pgoff_t start, pgoff_t end)
>>  static void notify_invalidate_page(struct inode *inode, struct folio *folio,
>>  				   pgoff_t start, pgoff_t end)
>>  {
>> -#ifdef CONFIG_MEMFILE_NOTIFIER
>>  	struct shmem_inode_info *info = SHMEM_I(inode);
>>  
>> +#ifdef CONFIG_MEMFILE_NOTIFIER
>>  	start = max(start, folio->index);
>>  	end = min(end, folio->index + folio_nr_pages(folio));
>>  
>>  	memfile_notifier_invalidate(&info->memfile_notifiers, start, end);
>>  #endif
>> +
>> +	if (info->xflags & SHM_F_INACCESSIBLE)
>> +		atomic64_sub(end - start, &current->mm->pinned_vm);
>
> As Vishal's to-be-posted selftest discovered, this is broken as 
> current->mm may
> be NULL.  Or it may be a completely different mm, e.g. AFAICT there's 
> nothing that
> prevents a different process from punching hole in the shmem backing.
>

How about just not charging the mm in the first place?  There’s precedent: ramfs and hugetlbfs (at least sometimes — I’ve lost track of the current status).

In any case, for an administrator to try to assemble the various rlimits into a coherent policy is, and always has been, quite messy. ISTM cgroup limits, which can actually add across processes usefully, are much better.

So, aside from the fact that these fds aren’t in a filesystem and are thus available by default, I’m not convinced that this accounting is useful or necessary.

Maybe we could just have some switch require to enable creation of private memory in the first place, and anyone who flips that switch without configuring cgroups is subject to DoS.