Hi Matthew.

Thanks for your comments and time to review the patchset.

On Mon, Oct 16, 2023 at 8:18 AM Matthew Wilcox willy@infradead.org wrote:

On Mon, Oct 16, 2023 at 02:38:19PM +0000, jeffxu@chromium.org wrote:

...

Modern CPUs support memory permissions such as RW and NX bits. Linux has supported NX since the release of kernel version 2.6.8 in August 2004 [1].

This seems like a confusing way to introduce the subject. Here, you're talking about page permissions, whereas (as far as I can tell), mseal() is about making _virtual_ addresses immutable, for some value of immutable.

...

Memory sealing additionally protects the mapping itself against modifications. This is useful to mitigate memory corruption issues where a corrupted pointer is passed to a memory management syscall. For example, such an attacker primitive can break control-flow integrity guarantees since read-only memory that is supposed to be trusted can become writable or .text pages can get remapped. Memory sealing can automatically be applied by the runtime loader to seal .text and .rodata pages and applications can additionally seal security critical data at runtime. A similar feature already exists in the XNU kernel with the VM_FLAGS_PERMANENT [3] flag and on OpenBSD with the mimmutable syscall [4]. Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

This [2] seems very generic and wide-ranging, not helpful. [5] was more useful to understand what you're trying to do.

...

The new mseal() is an architecture independent syscall, and with following signature:

mseal(void addr, size_t len, unsigned int types, unsigned int flags)

addr/len: memory range. Must be continuous/allocated memory, or else mseal() will fail and no VMA is updated. For details on acceptable arguments, please refer to comments in mseal.c. Those are also fully covered by the selftest.

Mmm. So when you say "continuous/allocated" what you really mean is "Must have contiguous VMAs" rather than "All pages in this range must be populated", yes?

There can't be a gap (unallocated memory) in the given range. Those are covered in selftest: test_seal_unmapped_start() test_seal_unmapped_middle() test_seal_unmapped_end() The comments in check_mm_seal() also mentioned that.

...

types: bit mask to specify which syscall to seal, currently they are: MM_SEAL_MSEAL 0x1 MM_SEAL_MPROTECT 0x2 MM_SEAL_MUNMAP 0x4 MM_SEAL_MMAP 0x8 MM_SEAL_MREMAP 0x10

I don't understand why we want this level of granularity. The OpenBSD and XNU examples just say "This must be immutable*". For values of immutable that allow downgrading access (eg RW to RO or RX to RO), but not upgrading access (RW->RX, RO->*, RX->RW).

...

Each bit represents sealing for one specific syscall type, e.g. MM_SEAL_MPROTECT will deny mprotect syscall. The consideration of bitmask is that the API is extendable, i.e. when needed, the sealing can be extended to madvise, mlock, etc. Backward compatibility is also easy.

Honestly, it feels too flexible. Why not just two flags to mprotect() -- PROT_IMMUTABLE and PROT_DOWNGRADABLE. I can see a use for that -- maybe for some things we want to be able to downgrade and for other things, we don't.

Having a seal type per syscall type helps to add the feature incrementally. Applications also know exactly what is sealed.

I'm not against types such as IMMUTABLE and DOWNGRADEABLE, if we can define what it seals precisely. As Jann pointed out, there have other scenarios that potentially affect IMMUTABLE. Implementing all thoses will take time. And if we missed a case, we could introduce backward compatibility issues to the application. Bitmask will solve this nicely, i.e. application will need to apply the newly added sealing type explicitly.

I'd like to see some discussion of how this interacts with mprotect(). As far as I can tell, the intent is to lock the protections/existance of the mapping, and not to force memory to stay in core. So it's fine for the kernel to swap out the page and set up a PTE as a swap entry. It's also fine for the kernel to mark PTEs as RO to catch page faults; we're concerned with the LOGICAL permissions, and not the page tables.

Yes. That is correct.

-Jeff Xu

On Mon, Oct 16, 2023 at 4:18 PM Matthew Wilcox willy@infradead.org wrote:

On Mon, Oct 16, 2023 at 02:38:19PM +0000, jeffxu@chromium.org wrote:

...

Modern CPUs support memory permissions such as RW and NX bits. Linux has supported NX since the release of kernel version 2.6.8 in August 2004 [1].

This seems like a confusing way to introduce the subject. Here, you're talking about page permissions, whereas (as far as I can tell), mseal() is about making _virtual_ addresses immutable, for some value of immutable.

...

Memory sealing additionally protects the mapping itself against modifications. This is useful to mitigate memory corruption issues where a corrupted pointer is passed to a memory management syscall. For example, such an attacker primitive can break control-flow integrity guarantees since read-only memory that is supposed to be trusted can become writable or .text pages can get remapped. Memory sealing can automatically be applied by the runtime loader to seal .text and .rodata pages and applications can additionally seal security critical data at runtime. A similar feature already exists in the XNU kernel with the VM_FLAGS_PERMANENT [3] flag and on OpenBSD with the mimmutable syscall [4]. Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

This [2] seems very generic and wide-ranging, not helpful. [5] was more useful to understand what you're trying to do.

...

The new mseal() is an architecture independent syscall, and with following signature:

mseal(void addr, size_t len, unsigned int types, unsigned int flags)

addr/len: memory range. Must be continuous/allocated memory, or else mseal() will fail and no VMA is updated. For details on acceptable arguments, please refer to comments in mseal.c. Those are also fully covered by the selftest.

Mmm. So when you say "continuous/allocated" what you really mean is "Must have contiguous VMAs" rather than "All pages in this range must be populated", yes?

...

types: bit mask to specify which syscall to seal, currently they are: MM_SEAL_MSEAL 0x1 MM_SEAL_MPROTECT 0x2 MM_SEAL_MUNMAP 0x4 MM_SEAL_MMAP 0x8 MM_SEAL_MREMAP 0x10

I don't understand why we want this level of granularity. The OpenBSD and XNU examples just say "This must be immutable*". For values of immutable that allow downgrading access (eg RW to RO or RX to RO), but not upgrading access (RW->RX, RO->*, RX->RW).

...

Each bit represents sealing for one specific syscall type, e.g. MM_SEAL_MPROTECT will deny mprotect syscall. The consideration of bitmask is that the API is extendable, i.e. when needed, the sealing can be extended to madvise, mlock, etc. Backward compatibility is also easy.

Honestly, it feels too flexible. Why not just two flags to mprotect() -- PROT_IMMUTABLE and PROT_DOWNGRADABLE. I can see a use for that -- maybe for some things we want to be able to downgrade and for other things, we don't.

I think it's worth pointing out that this suggestion (with PROT_*) could easily integrate with mmap() and as such allow for one-shot mmap() + mseal(). If we consider the common case as 'addr = mmap(...); mseal(addr);', it definitely sounds like a performance win as we halve the number of syscalls for a sealed mapping. And if we trivially look at e.g OpenBSD ld.so code, mmap() + mimmutable() and mprotect() + mimmutable() seem like common patterns.

On Tue, Oct 17, 2023 at 10:34 PM Jeff Xu jeffxu@google.com wrote:

On Tue, Oct 17, 2023 at 8:30 AM Pedro Falcato pedro.falcato@gmail.com wrote:

...

On Mon, Oct 16, 2023 at 4:18 PM Matthew Wilcox willy@infradead.org wrote:

...

On Mon, Oct 16, 2023 at 02:38:19PM +0000, jeffxu@chromium.org wrote:

...

Modern CPUs support memory permissions such as RW and NX bits. Linux has supported NX since the release of kernel version 2.6.8 in August 2004 [1].

This seems like a confusing way to introduce the subject. Here, you're talking about page permissions, whereas (as far as I can tell), mseal() is about making _virtual_ addresses immutable, for some value of immutable.

...

Memory sealing additionally protects the mapping itself against modifications. This is useful to mitigate memory corruption issues where a corrupted pointer is passed to a memory management syscall. For example, such an attacker primitive can break control-flow integrity guarantees since read-only memory that is supposed to be trusted can become writable or .text pages can get remapped. Memory sealing can automatically be applied by the runtime loader to seal .text and .rodata pages and applications can additionally seal security critical data at runtime. A similar feature already exists in the XNU kernel with the VM_FLAGS_PERMANENT [3] flag and on OpenBSD with the mimmutable syscall [4]. Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

This [2] seems very generic and wide-ranging, not helpful. [5] was more useful to understand what you're trying to do.

...

The new mseal() is an architecture independent syscall, and with following signature:

mseal(void addr, size_t len, unsigned int types, unsigned int flags)

addr/len: memory range. Must be continuous/allocated memory, or else mseal() will fail and no VMA is updated. For details on acceptable arguments, please refer to comments in mseal.c. Those are also fully covered by the selftest.

Mmm. So when you say "continuous/allocated" what you really mean is "Must have contiguous VMAs" rather than "All pages in this range must be populated", yes?

...

types: bit mask to specify which syscall to seal, currently they are: MM_SEAL_MSEAL 0x1 MM_SEAL_MPROTECT 0x2 MM_SEAL_MUNMAP 0x4 MM_SEAL_MMAP 0x8 MM_SEAL_MREMAP 0x10

I don't understand why we want this level of granularity. The OpenBSD and XNU examples just say "This must be immutable*". For values of immutable that allow downgrading access (eg RW to RO or RX to RO), but not upgrading access (RW->RX, RO->*, RX->RW).

...

Each bit represents sealing for one specific syscall type, e.g. MM_SEAL_MPROTECT will deny mprotect syscall. The consideration of bitmask is that the API is extendable, i.e. when needed, the sealing can be extended to madvise, mlock, etc. Backward compatibility is also easy.

Honestly, it feels too flexible. Why not just two flags to mprotect() -- PROT_IMMUTABLE and PROT_DOWNGRADABLE. I can see a use for that -- maybe for some things we want to be able to downgrade and for other things, we don't.

I think it's worth pointing out that this suggestion (with PROT_*) could easily integrate with mmap() and as such allow for one-shot mmap() + mseal(). If we consider the common case as 'addr = mmap(...); mseal(addr);', it definitely sounds like a performance win as we halve the number of syscalls for a sealed mapping. And if we trivially look at e.g OpenBSD ld.so code, mmap() + mimmutable() and mprotect() + mimmutable() seem like common patterns.

Yes. mmap() can support sealing as well, and memory is allocated as immutable from begining. This is orthogonal to mseal() though.

I don't see how this can be orthogonal to mseal(). In the case we opt for adding PROT_ bits, we should more or less only need to adapt calc_vm_prot_bits(), and the rest should work without issues. vma merging won't merge vmas with different prots. The current interfaces (mmap and mprotect) would work just fine. In this case, mseal() or mimmutable() would only be needed if you need to set immutability over a range of VMAs with different permissions.

Note: modifications should look kinda like this: https://godbolt.org/z/Tbjjd14Pe The only annoying wrench in my plans here is that we have effectively run out of vm_flags bits in 32-bit architectures, so this approach as I described is not compatible with 32-bit.

In case of ld.so, iiuc, memory can be first allocated as W, then later changed to RO, for example, during symbol resolution. The important point is that the application can decide what type of sealing it wants, and when to apply it. There needs to be an api(), that can be mseal() or mprotect2() or mimmutable(), the naming is not important to me.

mprotect() in linux have the following signature: int mprotect(void addr[.len], size_t len, int prot); the prot bitmasks are all taken here. I have not checked the prot field in mmap(), there might be bits left, even not, we could have mmap2(), so that is not an issue.

I don't see what you mean. We have plenty of prot bits left (32-bits, and we seem to have around 8 different bits used). And even if we didn't, prot is the same in mprotect and mmap and mmap2 :)

The only issue seems to be that 32-bit ran out of vm_flags, but that can probably be worked around if need be.

Hi Pedro

Some followup on mmap() + mprotect():

On Wed, Oct 18, 2023 at 11:20 AM Jeff Xu jeffxu@google.com wrote:

On Tue, Oct 17, 2023 at 3:35 PM Pedro Falcato pedro.falcato@gmail.com wrote:

...

...

...

I think it's worth pointing out that this suggestion (with PROT_*) could easily integrate with mmap() and as such allow for one-shot mmap() + mseal(). If we consider the common case as 'addr = mmap(...); mseal(addr);', it definitely sounds like a performance win as we halve the number of syscalls for a sealed mapping. And if we trivially look at e.g OpenBSD ld.so code, mmap() + mimmutable() and mprotect() + mimmutable() seem like common patterns.

Yes. mmap() can support sealing as well, and memory is allocated as immutable from begining. This is orthogonal to mseal() though.

I don't see how this can be orthogonal to mseal(). In the case we opt for adding PROT_ bits, we should more or less only need to adapt calc_vm_prot_bits(), and the rest should work without issues. vma merging won't merge vmas with different prots. The current interfaces (mmap and mprotect) would work just fine. In this case, mseal() or mimmutable() would only be needed if you need to set immutability over a range of VMAs with different permissions.

Agreed. By orthogonal, I meant we can have two APIs: mmap() and mseal()/mprotect() i.e. we can't just rely on mmap() only without mseal()/mprotect()/mimmutable(). Sealing can be applied after initial memory creation.

...

Note: modifications should look kinda like this: https://godbolt.org/z/Tbjjd14Pe The only annoying wrench in my plans here is that we have effectively run out of vm_flags bits in 32-bit architectures, so this approach as I described is not compatible with 32-bit.

...

In case of ld.so, iiuc, memory can be first allocated as W, then later changed to RO, for example, during symbol resolution. The important point is that the application can decide what type of sealing it wants, and when to apply it. There needs to be an api(), that can be mseal() or mprotect2() or mimmutable(), the naming is not important to me.

mprotect() in linux have the following signature: int mprotect(void addr[.len], size_t len, int prot); the prot bitmasks are all taken here. I have not checked the prot field in mmap(), there might be bits left, even not, we could have mmap2(), so that is not an issue.

I don't see what you mean. We have plenty of prot bits left (32-bits, and we seem to have around 8 different bits used). And even if we didn't, prot is the same in mprotect and mmap and mmap2 :)

The only issue seems to be that 32-bit ran out of vm_flags, but that can probably be worked around if need be.

Ah, you are right about this. vm_flags is full, and prot in mprotect() is not. Apology that I was wrong previously and caused confusion.

There is a slight difference in the syntax of mprotect and mseal. Each time when mprotect() is called, the kernel takes all of RWX bits and updates vm_flags, In other words, the application sets/unset each RWX, and kernel takes it.

In the mseal() case, the kernel will remember which seal types were applied previously, and the application doesn’t need to repeat all existing seal types in the next mseal(). Once a seal type is applied, it can’t be unsealed.

So if we want to use mprotect() for sealing, developers need to think of sealing bits differently than the rest of prot bits. It is a different programming model, might or might not be an obvious concept to developers.

This probably doesn't matter much to developers. We can enforce the sealing bit to be the same as the rest of PROT bits. If mprotect() tries to unset sealing, it will fail.

There is a difference in input check and error handling as well. for mseal(), if a given address range has a gap (unallocated memory), or if one of VMA is sealed with MM_SEAL_SEAL flag, none of VMAs is updated. For mprotect(), some VMAs can be updated, till an error happens to a VMA.

This difference doesn't matter much.

For mprotect()/mmap(), is Linux implementation limited by POSIX ? This can be made backward compatible. If there is no objection to adding linux specific values in mmap() and mprotect(), This works for me.

Thanks for your input. -Jeff

On Thu, 19 Oct 2023 at 15:47, Pedro Falcato pedro.falcato@gmail.com wrote:

...

For mprotect()/mmap(), is Linux implementation limited by POSIX ?

No. POSIX works merely as a baseline that UNIX systems aim towards. You can (and very frequently do) extend POSIX interfaces (in fact, it's how most of POSIX was written, through sheer "design-by-committee" on a bunch of UNIX systems' extensions).

We can in extreme circumstances actually go further than that, and not only extend on POSIX requirements, but actively even violate them.

It does need a very good reason, though, but it has happened when POSIX requirements were simply actively wrong.

For example, at one point POSIX required

int accept(int s, struct sockaddr *addr, size_t *addrlen);

which was simply completely wrong. It's utter shite, and didn't actually match any reality.

The 'addrlen' parameter is 'int *', and POSIX suddenly trying to make it "size_t" was completely unacceptable.

So we ignored it, told POSIX people that they were full of sh*t, and they eventually fixed it in the next version (by introducing a "socklen_t" that had better be the same as "int").

So POSIX can simply be wrong.

Also, if it turns out that we had some ABI that wasn't POSIX-compatible, the whole "don't break user space" will take precedence over any POSIX concerns, and we will not "fix" our system call if people already use our old semantics.

So in that case, we generally end up with a new system call (or new flags) instead.

Or sometimes it just is such a small detail that nobody cares - POSIX also has a notion of documenting areas of non-conformance, and people who really care end up having notions like "conformance vs _strict_ conformance".

Linus

On Thu, Oct 19, 2023 at 3:47 PM Pedro Falcato pedro.falcato@gmail.com wrote:

On Thu, Oct 19, 2023 at 6:30 PM Jeff Xu jeffxu@google.com wrote:

...

Hi Pedro

Some followup on mmap() + mprotect():

On Wed, Oct 18, 2023 at 11:20 AM Jeff Xu jeffxu@google.com wrote:

...

On Tue, Oct 17, 2023 at 3:35 PM Pedro Falcato pedro.falcato@gmail.com wrote:

...

...

...

I think it's worth pointing out that this suggestion (with PROT_*) could easily integrate with mmap() and as such allow for one-shot mmap() + mseal(). If we consider the common case as 'addr = mmap(...); mseal(addr);', it definitely sounds like a performance win as we halve the number of syscalls for a sealed mapping. And if we trivially look at e.g OpenBSD ld.so code, mmap() + mimmutable() and mprotect() + mimmutable() seem like common patterns.

Yes. mmap() can support sealing as well, and memory is allocated as immutable from begining. This is orthogonal to mseal() though.

I don't see how this can be orthogonal to mseal(). In the case we opt for adding PROT_ bits, we should more or less only need to adapt calc_vm_prot_bits(), and the rest should work without issues. vma merging won't merge vmas with different prots. The current interfaces (mmap and mprotect) would work just fine. In this case, mseal() or mimmutable() would only be needed if you need to set immutability over a range of VMAs with different permissions.

Agreed. By orthogonal, I meant we can have two APIs: mmap() and mseal()/mprotect() i.e. we can't just rely on mmap() only without mseal()/mprotect()/mimmutable(). Sealing can be applied after initial memory creation.

...

Note: modifications should look kinda like this: https://godbolt.org/z/Tbjjd14Pe The only annoying wrench in my plans here is that we have effectively run out of vm_flags bits in 32-bit architectures, so this approach as I described is not compatible with 32-bit.

...

In case of ld.so, iiuc, memory can be first allocated as W, then later changed to RO, for example, during symbol resolution. The important point is that the application can decide what type of sealing it wants, and when to apply it. There needs to be an api(), that can be mseal() or mprotect2() or mimmutable(), the naming is not important to me.

mprotect() in linux have the following signature: int mprotect(void addr[.len], size_t len, int prot); the prot bitmasks are all taken here. I have not checked the prot field in mmap(), there might be bits left, even not, we could have mmap2(), so that is not an issue.

I don't see what you mean. We have plenty of prot bits left (32-bits, and we seem to have around 8 different bits used). And even if we didn't, prot is the same in mprotect and mmap and mmap2 :)

The only issue seems to be that 32-bit ran out of vm_flags, but that can probably be worked around if need be.

Ah, you are right about this. vm_flags is full, and prot in mprotect() is not. Apology that I was wrong previously and caused confusion.

There is a slight difference in the syntax of mprotect and mseal. Each time when mprotect() is called, the kernel takes all of RWX bits and updates vm_flags, In other words, the application sets/unset each RWX, and kernel takes it.

In the mseal() case, the kernel will remember which seal types were applied previously, and the application doesn’t need to repeat all existing seal types in the next mseal(). Once a seal type is applied, it can’t be unsealed.

So if we want to use mprotect() for sealing, developers need to think of sealing bits differently than the rest of prot bits. It is a different programming model, might or might not be an obvious concept to developers.

This probably doesn't matter much to developers. We can enforce the sealing bit to be the same as the rest of PROT bits. If mprotect() tries to unset sealing, it will fail.

Yep. Erroneous or malicious mprotects would all be caught. However, if we add a PROT_DOWNGRADEABLE (that could let you, lets say, mprotect() to less permissions or even downright munmap()) you'd want some care to preserve that bit when setting permissions.

...

...

There is a difference in input check and error handling as well. for mseal(), if a given address range has a gap (unallocated memory), or if one of VMA is sealed with MM_SEAL_SEAL flag, none of VMAs is updated. For mprotect(), some VMAs can be updated, till an error happens to a VMA.

This difference doesn't matter much.

For mprotect()/mmap(), is Linux implementation limited by POSIX ?

No. POSIX works merely as a baseline that UNIX systems aim towards. You can (and very frequently do) extend POSIX interfaces (in fact, it's how most of POSIX was written, through sheer "design-by-committee" on a bunch of UNIX systems' extensions).

...

This can be made backward compatible. If there is no objection to adding linux specific values in mmap() and mprotect(), This works for me.

Linux already has system-specific values for PROT_ (PROT_BTI, PROT_MTE, PROT_GROWSUP, PROT_GROWSDOWN, etc). Whether this is the right interface is another question. I do like it a lot, but there's of course value in being compatible with existing solutions (like mimmutable()).

Thanks Pedro for providing examples on mm extension to POSIX. This opens more design options on solving the sealing problem. I will take a few days to research design options.

-Jeff

-- Pedro

Linus Torvalds torvalds@linux-foundation.org wrote:

On Tue, 17 Oct 2023 at 02:08, Jeff Xu jeffxu@google.com wrote:

...

It is probably worth noting that I choose to check one and only one sealing type per syscall. i.e. munmap(2) checks MM_SEAL_MUNMAP only.

Yeah, this is wrong.

It's wrong exactly because other system calls will unmap things too.

Using mmap() to over-map something will unmap the old one.

Same goes for mremap() to move over an existing mapping.

So the whole "do things by the name of the system call" is not workable.

All that matters is what the system calls *do*, not what their name is.

I agree completely...

mseal() is a clone of mimmutable(2), but with an extremely over-complicated API based upon dubious arguments.

I designed mimmutable(2) [1] in OpenBSD, it took about a year to get all the components working correctly. There were many intermediate API during development, but in the end the API is simply:

int mimmutable(void *addr, size_t len);

The kernel code for mimmutable() traverses the specified VA range. In that range, it will find unmapped sub-regions (which are are ignored) and mapped sub-regions. For these mapped regions, it does not care what the permissions are, it just marks each sub-region as immutable.

Later on, when any VM operation request upon a VA range attempts to (1) change the permissions (2) to re-map on top (3) or dispose of the mapping, that operation is refused with errno EPERM. We don't care where the request comes from (ie. what system call). It is a behaviour of the VM system, when asked to act upon a VA sub-range mapping.

Very simple semantics.

The only case where the immutable marker is ignored is during address space teardown as a result of process termination.

In his submission of this API, Jeff Xu makes three claims I find dubious;

Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

I specifically designed mimmutable(2) with chrome in mind, and the chrome binary running on OpenBSD is full of immutable mappings. All the library regions automatically become immutable because ld.so can infer it and do the mimmutable calls for the right subregions.

So this chrome work has already been done by OpenBSD, and it is dead simple. During early development I thought mimmutable(2) would be called by applications or libraries, but I was dead wrong: 99.9% of calls are from ld.so, and no applications need to call it, these are the two exceptions:

In OpenBSD, mimmutable() is used in libc malloc() to lock-down some data structures at initialization time, so they canoot be attacked to create an invariant for use in ROP return-to-libc style methods.

In Chrome, there is a v8_flags variable rounded out to a full page, and placed in .data. Chrome initialized this variable, and wants to mprotect PROT_READ, but .data has been made immutable by ld.so. So we force this page into a new ELF section called "openbsd.mutable" which also behaves RW like .data. Where chrome does the mprotect PROT_READ, it now also performs mimmutable() on that page.

Having a seal type per syscall type helps to add the feature incrementally.

Yet, somehow OpenBSD didn't do it per syscall, and we managed to make our entire base operating system and 10,000+ applications automatically receive the benefits. In one year's effort. The only application which cared about it was chrome, described in the previous paragraph.

I think Jeff's idea here is super dangerous. What will actually happen is people will add a few mseal() sub-operations and think the job is done. It isn't done. They need all the mseal() requests, or the mapping are not safe.

It is very counterproductive to provide developers a complex API that has insecure suboperations.

Applications also know exactly what is sealed.

Actually applicatins won't know because there is no tooling to inspect this -- but I will argue further that applications don't need to know. Immutable marking is a system decision, not a program decision.

I'll close by asking for a new look at the mimmutable(2) API we settled on for OpenBSD. I think there is nothing wrong with it. I'm willing to help guide glibc / ld.so / musl teams through the problems they may find along the way, I know where the skeletons are buried. Two in particular: -znow RELRO already today, and xonly-text in the future.

[1] https://man.openbsd.org/mimmutable.2

Linus Torvalds torvalds@linux-foundation.org wrote:

On Tue, 17 Oct 2023 at 11:20, Theo de Raadt deraadt@openbsd.org wrote:

...

The only case where the immutable marker is ignored is during address space teardown as a result of process termination.

.. and presumably also execve()?

Ah yes of course that also.

I do like us starting with just "mimmutable()", since it already exists. Particularly if chrome already knows how to use it.

Well, our chrome fork knows how to use it. Robert Nagy in our group maintains 1280 patches to make chrome work on OpenBSD. Google ignores them and will not upstream them. Some of these changes are security related, and they ignore them. Other changes are to cope with security work we've done on our own, for example: JIT changes from Stephen@google for mandatory IBT which google hasn't upstreamed yet, impacts due to PROT_EXEC-only mappings, etc.

But the only chrome diff required for mimmutable is for that v8_flags thing I described. And the same issue would need handling for mseal(). Introducing the new "mutable" ELF section is probably going to be a bigger fuss than the system call after mprotect(PROT_READ)....

Maybe add a flag field (require it to be zero initially) just to allow any future expansion. Maybe the chrome team has *wanted* to have some finer granularity thing and currently doesn't use mimmutable() in some case?

There's only one feature I can think of, and we already do it right now, documented in our manual page:

CAVEATS At present, mprotect(2) may reduce permissions on immutable pages marked PROT_READ | PROT_WRITE to the less permissive PROT_READ. This one-way operation is permitted for an introductory period to observe how software uses this mechanism. It may change to require explicit mutable region annotation with __attribute__((section(".openbsd.mutable"))) and explicit calls to mimmutable().

We had something which needed this behaviour during the development transition. It is exlusively mprotect RW -> R, no other transitions allowed.

But once the transition was done, we don't need it anymore. I want to delete it, because it is a bit of a trap. It still fails closed from an attack perspective, but...

What worries me is a piece of code reached by mistake can do a mprotect(lowering), not receive -1 EPERM, and carry on running.. I'd prefer the first time you touch a mapping in the wrong way, you receive indication of error. This only applies applies to mprotect() acting up on a region so the argument is a bit weak, due to mprotect() return value checking being about as rare as unicorns.

Also, it would be a pain for OpenBSD to transition to adding a 0 flag. I would need to see real cause not theory.

Stephen Röttger sroettger@google.com wrote:

...

I do like us starting with just "mimmutable()", since it already exists. Particularly if chrome already knows how to use it.

Maybe add a flag field (require it to be zero initially) just to allow any future expansion. Maybe the chrome team has *wanted* to have some finer granularity thing and currently doesn't use mimmutable() in some case?

Yes, we do have a use case in Chrome to split the sealing into unmap and mprotect which will allow us to seal additional pages that we can't seal with pure mimmutable().

For example, we have pkey-tagged RWX memory that we want to seal. Since the memory is already RWX and the pkey controls write access, we don't care about permission changes but sometimes we do need to mprotect data only pages.

Let me try to decompose this statement.

This is clearly for the JIT. You can pivot between the a JIT generated code mapping being RW and RX (or X-only), the object will pivot between W or X to satisfy W^X policy for safety.

I think you are talking about a RWX MAP_ANON object.

Then you use pkey_alloc() to get a PKEY. pkey_mprotect() sets the PKEY on the region. I argue you can then make it entirely immutable / sealed.

Let's say it is fully immutable / sealed.

After which, you can change the in-processor PKU register (using pkey_set) to toggle the Write-Inhibit and eXecute-Inhibit bits on that key.

The immutable object has a dangerous RWX permission. But the specific PKEY making it either RX (or X-only) or RW depending upon your context. The mapping is never exposed as RWX. The PKU model reduces the permission access of the object below the immutable permission level.

The security depends on the PKEY WI/XI bits being difficult to control.

SADLY on x86, this is managed with a PKRU userland register which is changeble without any supervisor control -- yes, it seems quite dangerous. Changing it requires a complicated MSR dance. It is unfortunate that the pkey_set() library function is easily reachedable in the PLT via ROP methods. On non-x86 cpus that have similar functionality, the register is privileged, but operating supporting it generally change it and return immediately.

The problem you seem to have with fully locked mseal() in chrome seems to be here:

about permission changes but sometimes we do need to mprotect data only pages.

Does that data have to be in the same region? Can your allocator not put the non-code pieces of the JIT elsewhere, with a different permission, fully immutable / msealed -- and perhaps even managed with a different PKEY if neccessary?

May that requires a huge rearchitecture. But isn't the root problem here that data and code are being handled in the same object with a shared permission model?

But the munmap sealing will provide protection against implicit changes of the pkey in this case which would happen if a page gets unmapped and another mapped in its place.

That primitive feels so weird, I have a difficult time believing it will remain unattackable in the long term.

But what if you could replace mprotect() with pkey_mprotect() upon a different key.. ?

--

A few more notes comparing what OpenBSD has done compared to Linux:

In OpenBSD, we do not have the pkey library. We have stolen one of the PKEY and use it for kernel support of xonly for kernel code and userland code. On x86 we recognize that userland can flip the permission by whacking the RPKU register -- which would make xonly code readable. (The chrome data you are trying to guard faces the same problem).

To prevent that, a majority of traps in the kernel (page faults, interrupts, etc) check if the PKRU register has been modified, and kill the process. It is statistically strong.

We are not making pkey available as a userland feature, but if we later do so we would still have 15 pkeys to play with. We would probably make the pkey_set() operation a system call, so the trap handler can also observe RPKU register modifications by the instruction.

Above, I mentioned pivoting between "RW or RX (or X-only)". On OpenBSD, chrome would be able to pivot between RW and X-only.

When it comes to Pkey utilization, we've ended up in a very different place than Linux.

On Tue, Oct 17, 2023 at 11:20 AM Theo de Raadt deraadt@openbsd.org wrote:

Linus Torvalds torvalds@linux-foundation.org wrote:

...

On Tue, 17 Oct 2023 at 02:08, Jeff Xu jeffxu@google.com wrote:

...

It is probably worth noting that I choose to check one and only one sealing type per syscall. i.e. munmap(2) checks MM_SEAL_MUNMAP only.

Yeah, this is wrong.

It's wrong exactly because other system calls will unmap things too.

Using mmap() to over-map something will unmap the old one.

Same goes for mremap() to move over an existing mapping.

So the whole "do things by the name of the system call" is not workable.

All that matters is what the system calls *do*, not what their name is.

I agree completely...

mseal() is a clone of mimmutable(2), but with an extremely over-complicated API based upon dubious arguments.

I designed mimmutable(2) [1] in OpenBSD, it took about a year to get all the components working correctly. There were many intermediate API during development, but in the end the API is simply:

 int mimmutable(void *addr, size_t len);

The kernel code for mimmutable() traverses the specified VA range. In that range, it will find unmapped sub-regions (which are are ignored) and mapped sub-regions. For these mapped regions, it does not care what the permissions are, it just marks each sub-region as immutable.

Later on, when any VM operation request upon a VA range attempts to (1) change the permissions (2) to re-map on top (3) or dispose of the mapping, that operation is refused with errno EPERM. We don't care where the request comes from (ie. what system call). It is a behaviour of the VM system, when asked to act upon a VA sub-range mapping.

Very simple semantics.

The only case where the immutable marker is ignored is during address space teardown as a result of process termination.

May I ask, for BSD's implementation of immutable(), do you cover things such as mlock(), madvice() ? or just the protection bit (WRX) + remap() + unmap().

In other words: Is BSD's definition of immutable equivalent to MM_SEAL_MPROTECT|MM_SEAL_MUNMAP|MM_SEAL_MREMAP|MM_SEAL_MMAP, of this patch set ?

I hesitate to introduce the concept of immutable into linux because I don't know all the scenarios present in linux where VMAs's metadata can be modified. As Jann's email pointed out, There could be quite a few things we still need to deal with, to completely block the possibility, e.g. malicious code attempting to write to a RO memory or change RW memory to RWX.

If, as part of immutable, I also block madvice(), mlock(), which also updates VMA's metadata, so by definition, I could. What if the user wants the features in madvice() and at the same time, also wants their .text protected ?

Also, if linux introduces a new syscall that depends on a new metadata of VMA, say msecret(), (for discussion purpose), should immutable automatically support that ?

Without those questions answered, I couldn't choose the route of immutable() yet.

-Jeff

In his submission of this API, Jeff Xu makes three claims I find dubious;

...

Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

I specifically designed mimmutable(2) with chrome in mind, and the chrome binary running on OpenBSD is full of immutable mappings. All the library regions automatically become immutable because ld.so can infer it and do the mimmutable calls for the right subregions.

So this chrome work has already been done by OpenBSD, and it is dead simple. During early development I thought mimmutable(2) would be called by applications or libraries, but I was dead wrong: 99.9% of calls are from ld.so, and no applications need to call it, these are the two exceptions:

In OpenBSD, mimmutable() is used in libc malloc() to lock-down some data structures at initialization time, so they canoot be attacked to create an invariant for use in ROP return-to-libc style methods.

In Chrome, there is a v8_flags variable rounded out to a full page, and placed in .data. Chrome initialized this variable, and wants to mprotect PROT_READ, but .data has been made immutable by ld.so. So we force this page into a new ELF section called "openbsd.mutable" which also behaves RW like .data. Where chrome does the mprotect PROT_READ, it now also performs mimmutable() on that page.

...

Having a seal type per syscall type helps to add the feature incrementally.

Yet, somehow OpenBSD didn't do it per syscall, and we managed to make our entire base operating system and 10,000+ applications automatically receive the benefits. In one year's effort. The only application which cared about it was chrome, described in the previous paragraph.

I think Jeff's idea here is super dangerous. What will actually happen is people will add a few mseal() sub-operations and think the job is done. It isn't done. They need all the mseal() requests, or the mapping are not safe.

It is very counterproductive to provide developers a complex API that has insecure suboperations.

...

Applications also know exactly what is sealed.

Actually applicatins won't know because there is no tooling to inspect this -- but I will argue further that applications don't need to know. Immutable marking is a system decision, not a program decision.

I'll close by asking for a new look at the mimmutable(2) API we settled on for OpenBSD. I think there is nothing wrong with it. I'm willing to help guide glibc / ld.so / musl teams through the problems they may find along the way, I know where the skeletons are buried. Two in particular: -znow RELRO already today, and xonly-text in the future.

[1] https://man.openbsd.org/mimmutable.2

On Tue, Oct 17, 2023 at 4:57 PM Theo de Raadt deraadt@openbsd.org wrote:

Jeff Xu jeffxu@google.com wrote:

...

May I ask, for BSD's implementation of immutable(), do you cover things such as mlock(), madvice() ? or just the protection bit (WRX) + remap() + unmap().

It only prevents removal of the mapping, placement of a replacement mapping, or changing the existing permissions. If one page in the existing sub-region is marked immutable, the whole operation fails with EPERM.

Those are the only user-visible aspects that an attacker cares about to utilize in this area.

mlock() and madvise() deal with the physical memory handling underneath the VA. They have nothing to do with how attack code might manipulate the VA address space inside a program to convert a series of dead-end approaches into a succesfull escalation strategy.

[It would be very long conversation to explain where and how this has been utilized to make an attack succesfull]

...

In other words: Is BSD's definition of immutable equivalent to MM_SEAL_MPROTECT|MM_SEAL_MUNMAP|MM_SEAL_MREMAP|MM_SEAL_MMAP, of this patch set ?

I can't compare it to your subsystem, because I completely fail to understand the cause or benefit of all the complexity.

I think I've explained what mimmutable() is in extremely simple terms.

Thanks for the explanation, based on those, this is exactly what the current set of patch does. In practice: libc could do below: #define MM_IMMUTABLE (MM_SEAL_MPROTECT|MM_SEAL_MUNMAP|MM_SEAL_MREMAP|MM_SEAL_MMAP) mseal(add,len, MM_IMMUTABLE) it will be equivalent to BSD's immutable().

And I don't understand else you are trying to do anything beyond what

mimmutable() offers. It seems like this is inventing additional solutions without proof that any of them are necessary to solve the specific problem that is known.

...

I hesitate to introduce the concept of immutable into linux because I don't know all the scenarios present in linux where VMAs's metadata can be modified.

Good grief. It seems obvious if you want to lock the change-behaviour of an object (the object in this case being a VA sub-region, there is a datastructure for that, in OpenBSD it is called an "entry"), then you put a flag in that object's data-structure and you simply check the flag everytime a change-operation is attempted. It is a flag which gets set, and checked. Nothing ever clears it (except address space teardown).

This flag must be put on the data structure that manages VA sub-ranges.

In our case when a prot/mapping operation reaches low-level code that will want to change an "entry", we notice it is not allowed and simply percolate EPERM up through the layers.

...

There could be quite a few things we still need to deal with, to completely block the possibility, e.g. malicious code attempting to write to a RO memory

What?! writes to RO memory are blocked by the permission bits.

...

or change RW memory to RWX.

In our case that is blocked by W^X policy.

But if the region is marked mimmutable, then that's another reason you cannot change RW to RWX. It seems so off-topic, to talk about writes to RO memory. I get a feeling you are a bit lost.

immutable() is not about permissions, but about locking permissions.

• You can't change the permissions of the address space region.
• You cannot map a replacement object at the location instead (especially with different permission).
• You cannot unmap at that location (which you would do if you wanted to map a new object, with a different permission).

All 3 of these scenarios are identical. No regular code performs these 3 operations on regions of the address space which we mark immutable.

There is nothing more to mimmutable in the VM layer. The hard work is writing code in execve() and ld.so which will decide which objects can be marked immutable automatically, so that programs don't do this to themselves.

I'm aware of where this simple piece fits in. It does not solve all problems, it is a very narrow change to impact a problem which only high-value targets will ever face (like chrome).

But I think you don't understand the purpose of this mechanism.

In linux cases, I think, eventually, mseal() will have a bigger scope than BSD's mimmutable(). VMA's metadata(vm_area_struct) contains a lot of control info, depending on application's needs, mseal() can be expanded to seal individual control info.

For example, in madvice(2) case: As Jann point out in [1] and I quote: "you'd probably also want to block destructive madvise() operations that can effectively alter region contents by discarding pages and such, ..."

Another example: if an application wants to keep a memory always present in RAM, for whatever the reason, it can call seal the mlock().

To handle those two new cases. mseal() could add two more bits: MM_SEAL_MADVICE, MM_SEAL_MLOCK.

It is practical to keep syscall extentable, when the business logic is the same.

I think I explained the logic of using bitmasks in the mseal() interface clearly with the example of madvice() and mlock().

-Jeff

[1] https://lore.kernel.org/lkml/CAG48ez3ShUYey+ZAFsU2i1RpQn0a5eOs2hzQ426FkcgnfU...

...

If, as part of immutable, I also block madvice(), mlock(), which also updates VMA's metadata, so by definition, I could. What if the user wants the features in madvice() and at the same time, also wants their .text protected ?

I have no idea what you are talking about. None of those things relate to the access permission of the memory the user sees, and therefore none of them are in the attack surface profile which is being prevented.

Meaning, we allow madvise() and mlock() and mphysicalquantummemory() because those relate to the physical storage and not the VA permission model.

...

Also, if linux introduces a new syscall that depends on a new metadata of VMA, say msecret(), (for discussion purpose), should immutable automatically support that ?

How about the future makingexcuses() system call?

I don't think you understand the problem space well enough to come up with your own solution for it. I spent a year on this, and ship a complete system using it. You are asking such simplistic questions above it shocks me.

Maybe read the LWN article;

https://lwn.net/Articles/915640/

...

Without those questions answered, I couldn't choose the route of immutable() yet.

"... so I can clearly not choose the wine in front of you."

If you don't understand what this thing is for, and cannot minimize the complexity of this thing, then Linux doesn't need it at all.

I should warn everyone the hard work is not in the VM layer, but in ld.so -- deciding which parts of the image to make immutable, and when. It is also possible to make some segments immutable directly in execve() -- but in both cases you better have a really good grasp on RELRO executable layout or will make too many pieces immutable...

I am pretty sure Linux will never get as far as we got. Even our main stacks are marked immutable, but in Linux that would conflict with glibc ld.so mprotecting RWX the stack if you dlopen() a shared library with GNUSTACK, a very bad idea which needs a different fight...

On Tue, Oct 17, 2023 at 08:18:47PM -0700, Jeff Xu wrote:

In practice: libc could do below: #define MM_IMMUTABLE (MM_SEAL_MPROTECT|MM_SEAL_MUNMAP|MM_SEAL_MREMAP|MM_SEAL_MMAP) mseal(add,len, MM_IMMUTABLE) it will be equivalent to BSD's immutable().

No, it wouldn't, because you've carefully listed the syscalls you're blocking instead of understanding the _concept_ of what you need to block.

In linux cases, I think, eventually, mseal() will have a bigger scope than BSD's mimmutable(). VMA's metadata(vm_area_struct) contains a lot of control info, depending on application's needs, mseal() can be expanded to seal individual control info.

For example, in madvice(2) case: As Jann point out in [1] and I quote: "you'd probably also want to block destructive madvise() operations that can effectively alter region contents by discarding pages and such, ..."

Another example: if an application wants to keep a memory always present in RAM, for whatever the reason, it can call seal the mlock().

To handle those two new cases. mseal() could add two more bits: MM_SEAL_MADVICE, MM_SEAL_MLOCK.

Yes, thank you for demonstrating that you have no idea what you need to block.

It is practical to keep syscall extentable, when the business logic is the same.

I concur with Theo & Linus. You don't know what you're doing. I think the underlying idea of mimmutable() is good, but how you've split it up and how you've implemented it is terrible.

Let's start with the purpose. The point of mimmutable/mseal/whatever is to fix the mapping of an address range to its underlying object, be it a particular file mapping or anonymous memory. After the call succeeds, it must not be possible to make any address in that virtual range point into any other object.

The secondary purpose is to lock down permissions on that range. Possibly to fix them where they are, possibly to allow RW->RO transitions.

With those purposes in mind, you should be able to deduce for any syscall or any madvise(), ... whether it should be allowed.

Look, I appreciate this is only your second set of patches to Linux and you've taken on a big job. But that's all the more reason you should listen to people who are trying to help you.

Jeff Xu jeffxu@google.com wrote:

On Wed, Oct 18, 2023 at 8:17 AM Matthew Wilcox willy@infradead.org wrote:

...

Let's start with the purpose. The point of mimmutable/mseal/whatever is to fix the mapping of an address range to its underlying object, be it a particular file mapping or anonymous memory. After the call succeeds, it must not be possible to make any address in that virtual range point into any other object.

The secondary purpose is to lock down permissions on that range. Possibly to fix them where they are, possibly to allow RW->RO transitions.

With those purposes in mind, you should be able to deduce for any syscall or any madvise(), ... whether it should be allowed.

I got it.

IMO: The approaches mimmutable() and mseal() took are different, but we all want to seal the memory from attackers and make the linux application safer.

I think you are building mseal for chrome, and chrome alone.

I do not think this will work out for the rest of the application space because

1) it is too complicated 2) experience with mimmutable() says that applications don't do any of it themselves, it is all in execve(), libc initialization, and ld.so. You don't strike me as an execve, libc, or ld.so developer.

Stephen Röttger sroettger@google.com wrote:

...

...

IMO: The approaches mimmutable() and mseal() took are different, but we all want to seal the memory from attackers and make the linux application safer.

I think you are building mseal for chrome, and chrome alone.

I do not think this will work out for the rest of the application space because

1. it is too complicated
2. experience with mimmutable() says that applications don't do any of it themselves, it is all in execve(), libc initialization, and ld.so. You don't strike me as an execve, libc, or ld.so developer.

We do want to build this in a way that it can be applied automatically by ld.so and we appreciate all your feedback on this.

Hi Stephen,

I am pretty sure your mechanism will be useable by ld.so.

What bothers me is the complex many-bits approach may encourage people to set only a subset of the bits, and then believe they have a security primitive.

Partial sealing is not safe. I define partial sealing as "blocking munmap, but not mprotect". Or "blocking mprotect, but not madvise or mmap".

In Message-id ZS/3GCKvNn5qzhC4@casper.infradead.org Matthew stated there that there are two aspects being locked: which object is mapped, and the permission of that mapping. When additional system calls msync() and madvise() are included in the picture, there are 3 actions being prevented:

- Can someone replace the object - Can someone change the permission - Can someone throw away the cached pages, reverting to original content of the object (that is the madvise / msync)

In Message-id: CAG48ez3ShUYey+ZAFsU2i1RpQn0a5eOs2hzQ426FkcgnfUGLvA@mail.gmail.com Jan reminded us of this piece. I'm taking this as a long-standing security hole in some sub-operations of msync/madvise which can write to data regions that aren't actually writeable. Sub-operations with this problem are MADV_FREE, MADV_DONTNEED, POSIX_MADV_DONTNEED, MS_INVALIDATE.. on Linux MADV_WIPEONFORK, and probably a whole bunch of others. I am testing OpenBSD changes which demand PROT_WRITE permission for these sub-operations. Perhaps some systems are already careful.

If you leave any of these operators available, the object is not actually sealed against abuse. I believe an attacker will simply switch to a different operator (mmap, munmap, mprotect, madvise, msync) to achieve a similar objective of damaging the permission or contents.

Since mseal() is designed to create partial sealings, the name of the proposed system call really smells.

The intention of splitting the sealing by syscall was to provide flexibility while still allowing ld.so to seal all operations.

Yes, you will have ld.so set all the bits, and the same in C runtime initialization. If you convince glibc to stop make the stack executable in dlopen(), the kernel could automatically do it.. With Linux backwards compat management, getting there would be an extremely long long long roadmap. But anyways the idea would be "set all the bits". Because otherwise the object or data isn't safe.

Does Linus' proposal to just split munmap / mprotect sealing address your complexity concerns? ld.so would always use both flags which should then behave similar to mimmutable().

No, I think it is weak, because it isn't sealed.

A seperate mail in the thread from you says this is about chrome wanting to use PKU on RWX objects. I think that's the reason for wanting to seperate the sealing (I haven't heard of other applications wanting that). How about we explore that in the other subthread..

Hi Jann,

Thank you for reviewing the patch and comments.

On Mon, Oct 16, 2023 at 10:35 AM Jann Horn jannh@google.com wrote:

On Mon, Oct 16, 2023 at 4:38 PM jeffxu@chromium.org wrote:

...

From: Jeff Xu jeffxu@google.com

This patchset proposes a new mseal() syscall for the Linux kernel.

Modern CPUs support memory permissions such as RW and NX bits. Linux has supported NX since the release of kernel version 2.6.8 in August 2004 [1]. The memory permission feature improves security stance on memory corruption bugs, i.e. the attacker can’t just write to arbitrary memory and point the code to it, the memory has to be marked with X bit, or else an exception will happen.

Memory sealing additionally protects the mapping itself against modifications. This is useful to mitigate memory corruption issues where a corrupted pointer is passed to a memory management syscall. For example, such an attacker primitive can break control-flow integrity guarantees since read-only memory that is supposed to be trusted can become writable or .text pages can get remapped. Memory sealing can automatically be applied by the runtime loader to seal .text and .rodata pages and applications can additionally seal security critical data at runtime. A similar feature already exists in the XNU kernel with the VM_FLAGS_PERMANENT [3] flag and on OpenBSD with the mimmutable syscall [4]. Also, Chrome wants to adopt this feature for their CFI work [2] and this patchset has been designed to be compatible with the Chrome use case.

The new mseal() is an architecture independent syscall, and with following signature:

mseal(void addr, size_t len, unsigned int types, unsigned int flags)

Is the plan that the VMAs you need to protect would be created and mseal()'ed while you expect that attacker code can not (yet) be running concurrently?

Yes.

Do you expect to be using sealed memory for shadow stacks (in x86 CET / arm64 GCS) to prevent an attacker from mixing those up by moving pages inside a shadow stack or between different shadow stacks or such? (If that's even possible, I think it is but I haven't tried.)

I will defer the question to Stephen. (I also think that is possible, if the application know where the shadow stack is at runtime)

...

addr/len: memory range. Must be continuous/allocated memory, or else mseal() will fail and no VMA is updated. For details on acceptable arguments, please refer to comments in mseal.c. Those are also fully covered by the selftest. types: bit mask to specify which syscall to seal, currently they are: MM_SEAL_MSEAL 0x1 MM_SEAL_MPROTECT 0x2 MM_SEAL_MUNMAP 0x4 MM_SEAL_MMAP 0x8 MM_SEAL_MREMAP 0x10

You'd probably also want to block destructive madvise() operations that can effectively alter region contents by discarding pages and such, in particular MADV_FREE, MADV_DONTNEED, MADV_DONTNEED_LOCKED; probably also MADV_REMOVE, MADV_DONTFORK, MADV_WIPEONFORK. Maybe you'd want to just block all madvise() for sealed VMAs? Or rename process_madvise_behavior_valid() to something like "madvise_is_nondestructive()" and use that.

Thanks for the suggestion. I can add madvise() later, maybe in another series. For now, the goal of this patchset covers 4 syscalls, as wished for by Chrome initially.

The following comments probably mostly don't matter in practice if this feature is used in a context that is heavily seccomp-sandboxed (like Desktop Linux Chrome), but should maybe be addressed to make this feature more usable for other users. (Including Android Chrome, which has a weaker sandbox...)

FWIW, it is also possible to write to read-only memory through the /proc/self/mem interface (or through ptrace commands like PTRACE_POKETEXT) because of FOLL_FORCE, depending on kernel configuration, seccomp policy, and what the LSMs do. (I think Android Chrome would allow /proc/self/mem writes, but would block PTRACE_POKETEXT with RestrictPtrace() in the sandbox code?)

I had a related ancient patch series in 2016 with an attempt to allow SELinux to prevent bypassing W^X protections with this, but I never followed through with getting that landed... (https://lore.kernel.org/linux-mm/1475103281-7989-1-git-send-email-jann@thejh...)

I guess the question there is what the right semantics for this kind of protected memory are when a debugger is active. The simple solution that might break some debugging would be "just deny all FOLL_FORCE write access for this memory" (which would prevent debuggers from being able to place breakpoints, which would maybe not be great). But maybe it makes more sense to consider this to be an independent concern and solve it with a new SELinux feature or something like that instead, and then document that mseal() requires some complement to prevent forced writes to read-only private memory? (For which the simplest solution would be "don't grant filesystem access or ptrace() access to the sandboxed code".)

What is the intended interaction with userfaultfd, which I believe by design permits arbitrary data into unpopulated areas of anonymous VMAs? If the intention is that the process should otherwise be sandboxed to not have access to userfaultfd, that should maybe be documented. (Alternatively I guess you could theoretically remove the VM_MAYWRITE bit from marked VMAs, but that might be more strict than we want, since it'd also block all FOLL_FORCE writes.)

There are also some interfaces like AIO or the X86 Shadow Stacks interface that indirectly unmap memory through the kernel and look like they could perhaps be tricked into racily unmapping a just-created sealed VMA. You'd probably have to make sure that they can't do that and essentially treat their unmap operations as if they came from userspace, I guess? What Linus just wrote.

I think either way this feature needs some documentation on what kind of context it's supposed to run in.

Thanks for listing all the cases. As you pointed out, Chrome is heavily sandboxed, with syscalls and file access. I will work with Stephan to check if some of these applied to Chrome.

It is probably worth to mention, with the current approach of mseal(), i.e. by bitmask, it is possible to implement those above incrementally.

Thanks -Jeff

-Jeff

...

Each bit represents sealing for one specific syscall type, e.g. MM_SEAL_MPROTECT will deny mprotect syscall. The consideration of bitmask is that the API is extendable, i.e. when needed, the sealing can be extended to madvise, mlock, etc. Backward compatibility is also easy.

The kernel will remember which seal types are applied, and the application doesn’t need to repeat all existing seal types in the next mseal(). Once a seal type is applied, it can’t be unsealed. Call mseal() on an existing seal type is a no-action, not a failure.

MM_SEAL_MSEAL will deny mseal() calls that try to add a new seal type.

Internally, vm_area_struct adds a new field vm_seals, to store the bit masks.

For the affected syscalls, such as mprotect, a check(can_modify_mm) for sealing is added, this usually happens at the early point of the syscall, before any update is made to VMAs. The effect of that is: if any of the VMAs in the given address range fails the sealing check, none of the VMA will be updated. It might be worth noting that this is different from the rest of mprotect(), where some updates can happen even when mprotect returns fail. Consider can_modify_mm only checks vm_seals in vm_area_struct, and it is not going deeper in the page table or updating any HW, success or none behavior might fit better here. I would like to listen to the community's feedback on this.

The idea that inspired this patch comes from Stephen Röttger’s work in V8 CFI [5], Chrome browser in ChromeOS will be the first user of this API.

In addition, Stephen is working on glibc change to add sealing support into the dynamic linker to seal all non-writable segments at startup. When that work is completed, all applications can automatically benefit from these new protections.

[1] https://kernelnewbies.org/Linux_2_6_8 [2] https://v8.dev/blog/control-flow-integrity [3] https://github.com/apple-oss-distributions/xnu/blob/1031c584a5e37aff177559b9... [4] https://man.openbsd.org/mimmutable.2 [5] https://docs.google.com/document/d/1O2jwK4dxI3nRcOJuPYkonhTkNQfbmwdvxQMyXgea...

Jeff Xu (8): Add mseal syscall Wire up mseal syscall mseal: add can_modify_mm and can_modify_vma mseal: seal mprotect mseal munmap mseal mremap mseal mmap selftest mm/mseal mprotect/munmap/mremap/mmap

arch/alpha/kernel/syscalls/syscall.tbl | 1 + arch/arm/tools/syscall.tbl | 1 + arch/arm64/include/asm/unistd.h | 2 +- arch/arm64/include/asm/unistd32.h | 2 + arch/ia64/kernel/syscalls/syscall.tbl | 1 + arch/m68k/kernel/syscalls/syscall.tbl | 1 + arch/microblaze/kernel/syscalls/syscall.tbl | 1 + arch/mips/kernel/syscalls/syscall_n32.tbl | 1 + arch/mips/kernel/syscalls/syscall_n64.tbl | 1 + arch/mips/kernel/syscalls/syscall_o32.tbl | 1 + arch/parisc/kernel/syscalls/syscall.tbl | 1 + arch/powerpc/kernel/syscalls/syscall.tbl | 1 + arch/s390/kernel/syscalls/syscall.tbl | 1 + arch/sh/kernel/syscalls/syscall.tbl | 1 + arch/sparc/kernel/syscalls/syscall.tbl | 1 + arch/x86/entry/syscalls/syscall_32.tbl | 1 + arch/x86/entry/syscalls/syscall_64.tbl | 1 + arch/xtensa/kernel/syscalls/syscall.tbl | 1 + fs/aio.c | 5 +- include/linux/mm.h | 55 +- include/linux/mm_types.h | 7 + include/linux/syscalls.h | 2 + include/uapi/asm-generic/unistd.h | 5 +- include/uapi/linux/mman.h | 6 + ipc/shm.c | 3 +- kernel/sys_ni.c | 1 + mm/Kconfig | 8 + mm/Makefile | 1 + mm/internal.h | 4 +- mm/mmap.c | 49 +- mm/mprotect.c | 6 + mm/mremap.c | 19 +- mm/mseal.c | 328 +++++ mm/nommu.c | 6 +- mm/util.c | 8 +- tools/testing/selftests/mm/Makefile | 1 + tools/testing/selftests/mm/mseal_test.c | 1428 +++++++++++++++++++ 37 files changed, 1934 insertions(+), 28 deletions(-) create mode 100644 mm/mseal.c create mode 100644 tools/testing/selftests/mm/mseal_test.c

-- 2.42.0.609.gbb76f46606-goog