linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Lorenzo Stoakes <ljs@kernel.org>
To: Usama Arif <usama.arif@linux.dev>
Cc: "Denis M. Karpov" <komlomal@gmail.com>,
	rppt@kernel.org,  akpm@linux-foundation.org,
	Liam.Howlett@oracle.com, vbabka@kernel.org, jannh@google.com,
	 peterx@redhat.com, pfalcato@suse.de, brauner@kernel.org,
	viro@zeniv.linux.org.uk,  jack@suse.cz, linux-mm@kvack.org,
	linux-fsdevel@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH] userfaultfd: allow registration of ranges below mmap_min_addr
Date: Thu, 9 Apr 2026 09:01:20 +0100	[thread overview]
Message-ID: <addcUpxfuR2llaiW@lucifer> (raw)
In-Reply-To: <20260408123700.1596800-1-usama.arif@linux.dev>

On Wed, Apr 08, 2026 at 05:36:59AM -0700, Usama Arif wrote:
> On Tue,  7 Apr 2026 11:14:42 +0300 "Denis M. Karpov" <komlomal@gmail.com> wrote:
>
> > The current implementation of validate_range() in fs/userfaultfd.c
> > performs a hard check against mmap_min_addr without considering
> > capabilities, but the mmap() syscall uses security_mmap_addr()
> > which allows privileged processes (with CAP_SYS_RAWIO) to map below
> > mmap_min_addr. Furthermore, security_mmap_addr()->cap_mmap_addr() uses
> > dac_mmap_min_addr variable which can be changed with
> > /proc/sys/vm/mmap_min_addr.
> >
> > Because userfaultfd uses a different check, UFFDIO_REGISTER may fail
> > with -EINVAL for valid memory areas that were successfully mapped
> > below mmap_min_addr even with appropriate capabilities.
> >
> > This prevents apps like binary compilers from using UFFD for valid memory
> > regions mapped by application.
> >
> > Replace the rigid mmap_min_addr check with security_mmap_addr() to align
> > userfaultfd with the standard kernel memory mapping security policy.
> >
> > Signed-off-by: Denis M. Karpov <komlomal@gmail.com>
> >
> > ---
> > Initial RFC following the discussion on the [BUG] thread.
> > Link: https://lore.kernel.org/all/CADtiZd0tWysx5HMCUnOXfSHB7PXAuXg1Mh4eY_hUmH29S=sejg@mail.gmail.com/
> > ---
> >  fs/userfaultfd.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> > index bdc84e521..dbfe5b2a0 100644
> > --- a/fs/userfaultfd.c
> > +++ b/fs/userfaultfd.c
> > @@ -1238,15 +1238,13 @@ static __always_inline int validate_unaligned_range(
> >  		return -EINVAL;
> >  	if (!len)
> >  		return -EINVAL;
> > -	if (start < mmap_min_addr)
> > -		return -EINVAL;
> >  	if (start >= task_size)
> >  		return -EINVAL;
> >  	if (len > task_size - start)
> >  		return -EINVAL;
> >  	if (start + len <= start)
> >  		return -EINVAL;
> > -	return 0;
> > +	return security_mmap_addr(start);
>
> Is this introducing an ABI change?
>
> The old code returned -EINVAL when start was below mmap_min_addr.
> The new code calls security_mmap_addr() which returns -EPERM when
> the caller lacks CAP_SYS_RAWIO. Existing userspace callers checking
> specifically for -EINVAL would see different behavior start is
> below mmap_min_addr.

You mean API change? :) we don't guarantee ABI for kernel stuff anyway.

Firstly, as with Harry, I don't believe we should be duplicating checks here
anyway. UFFD is duplicative enough as it is.

And this is such a silly edge case that I don't think it is valid or reasonable
for us to account for whichever totally insane user relies on a pointless
re-check being done there and _then_ relies on the error code
being... -EINVAL... which is overloaded for a million other possible failures.

Let's let it be -EFAULT and remove this silly check altogether.

>
> >  }
> >
> >  static __always_inline int validate_range(struct mm_struct *mm,
> > --
> > 2.47.3
> >
> >

Thanks, Lorenzo


  reply	other threads:[~2026-04-09  8:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-07  8:14 Denis M. Karpov
2026-04-08  3:21 ` Harry Yoo (Oracle)
2026-04-08  8:09   ` Denis M. Karpov
2026-04-09  2:51     ` Harry Yoo (Oracle)
2026-04-09  7:58       ` Lorenzo Stoakes
2026-04-08 12:36 ` Usama Arif
2026-04-09  8:01   ` Lorenzo Stoakes [this message]
2026-04-09  9:05     ` Denis M. Karpov
2026-04-09 10:52     ` Usama Arif

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=addcUpxfuR2llaiW@lucifer \
    --to=ljs@kernel.org \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=komlomal@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=peterx@redhat.com \
    --cc=pfalcato@suse.de \
    --cc=rppt@kernel.org \
    --cc=usama.arif@linux.dev \
    --cc=vbabka@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox