On Thu, Jan 09, 2025 at 03:30:36PM -0800, Jeff Xu wrote:
> On Wed, Jan 8, 2025 at 11:06 AM Lorenzo Stoakes
> <lorenzo.stoakes@xxxxxxxxxx> wrote:
> >
> > On Mon, Jan 06, 2025 at 04:44:33PM -0800, Kees Cook wrote:
> > > On Mon, Jan 06, 2025 at 10:26:27AM -0800, Jeff Xu wrote:
> > > > + Kees because this is related to W^X memfd and security.
> > > >
> > > > On Fri, Jan 3, 2025 at 7:14 AM Jann Horn <jannh@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Fri, Dec 6, 2024 at 7:19 PM Lorenzo Stoakes
> > > > > <lorenzo.stoakes@xxxxxxxxxx> wrote:
> > > > > > On Thu, Dec 05, 2024 at 05:09:22PM -0800, Isaac J. Manjarres wrote:
> > > > > > > + if (is_exec_sealed(seals)) {
> > > > > >
> > > > > > Are we intentionally disallowing a MAP_PRIVATE memfd's mapping's execution?
> > > > > > I've not tested this scenario so don't know if we somehow disallow this in
> > > > > > another way but note on write checks we only care about shared mappings.
> > > > > >
> > > > > > I mean one could argue that a MAP_PRIVATE situation is the same as copying
> > > > > > the data into an anon buffer and doing what you want with it, here you
> > > > > > could argue the same...
> > > > > >
> > > > > > So probably we should only care about VM_SHARED?
> > > > >
> > > > > FWIW I think it doesn't make sense to distinguish between
> > > > > shared/private mappings here - in the scenario described in the cover
> > > > > letter, it wouldn't matter that much to an attacker whether the
> > > > > mapping is shared or private (as long as the VMA contents haven't been
> > > > > CoWed already).
> > > > +1 on this.
> > > > The concept of blocking this for only shared mapping is questionable.
> > >
> > > Right -- why does sharedness matter? It seems more robust to me to not
> > > create a corner case but rather apply the flag/behavior universally?
> > >
> >
> > I'm struggling to understand what you are protecting against, if I can receive a
> > buffer '-not executable-'. But then copy it into another buffer I mapped, and
> > execute it?
> >
> preventing mmap() a memfd has the same threat model as preventing
> execve() of a memfd, using execve() of a memfd as an example (since
> the kernel already supports this): an attacker wanting to execute a
> hijacked memfd must already have the ability to call execve() (e.g.,
> by modifying a function pointer or using ROP). To prevent this, the
> kernel supports making memfds non-executable (rw-) and permanently
> preventing them from becoming executable (sealing with F_SEAL_EXEC).
> Once the execve() attack path is blocked, the next thing an attacker
> could do is mmap() the memfd into the process's memory and jump to it.
>
I think the main issue in the threat model that I described is that
an attacking process can gain control of a more priveleged process.
Yes, having the buffer sealed against execution would prevent the
attacker from running the injected from *that* buffer, but
if they're already controlling the process, they could have the process
create a memfd that is executable (imagine a system where
MFD_NOEXEC_SEAL is not the default), copy the code, and then execute it
from there.
I spoke about this offline with Jann as well, and we both agree that
given that line of reasoning, this feature that I'm trying to add
doesn't buy us the security that I initially thought it would.
Therefore, we will be dropping this patch.
Thank you everyone for the discussion and reviews!
--Isaac
