* Re: [PATCH v16 4/5] random: introduce generic vDSO getrandom() implementation
From: Andy Lutomirski @ 2024-05-31 23:06 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand
In-Reply-To: <20240528122352.2485958-5-Jason@zx2c4.com>
> On May 28, 2024, at 5:25 AM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Provide a generic C vDSO getrandom() implementation, which operates on
> an opaque state returned by vgetrandom_alloc() and produces random bytes
> the same way as getrandom(). This has a the API signature:
>
> ssize_t vgetrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state);
> +/**
> + * type vdso_kernel_ulong - unsigned long type that matches kernel's unsigned long
> + *
> + * Data shared between userspace and the kernel must operate the same way in both 64-bit code and in
> + * 32-bit compat code, over the same potentially 64-bit kernel. This type represents the size of an
> + * unsigned long as used by kernel code. This isn't necessarily the same as an unsigned long as used
> + * by userspace, however.
Why is this better than using plain u64? It’s certainly more
complicated. It also rather fundamentally breaks CRIU on 32-bit
userspace (although CRIU may well be unable to keep vgetrandom working
after a restore onto a different kernel anyway). Admittedly 32-bit
userspace is a slowly dying breed, but still.
> + *
> + * +-------------------+-------------------+------------------+-------------------+
> + * | 32-bit userspace | 32-bit userspace | 64-bit userspace | 64-bit userspace |
> + * | unsigned long | vdso_kernel_ulong | unsigned long | vdso_kernel_ulong |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + * | 32-bit kernel | ✓ same size | ✓ same size |
> + * | unsigned long | | |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + * | 64-bit kernel | ✘ different size! | ✓ same size | ✓ same size | ✓ same size |
> + * | unsigned long | | | | |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + */
> +#ifdef CONFIG_64BIT
> +typedef u64 vdso_kernel_ulong;
> +#else
> +typedef u32 vdso_kernel_ulong;
> +#endif
> +
> +#endif /* __VDSO_TYPES_H */
> diff --git a/lib/vdso/getrandom.c b/lib/vdso/getrandom.c
> new file mode 100644
> index 000000000000..4d9bb59985f8
> --- /dev/null
> +++ b/lib/vdso/getrandom.c
> @@ -0,0 +1,226 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <linux/cache.h>
> +#include <linux/kernel.h>
> +#include <linux/time64.h>
> +#include <vdso/datapage.h>
> +#include <vdso/getrandom.h>
> +#include <asm/vdso/getrandom.h>
> +#include <asm/vdso/vsyscall.h>
> +
> +#define MEMCPY_AND_ZERO_SRC(type, dst, src, len) do { \
> + while (len >= sizeof(type)) { \
> + __put_unaligned_t(type, __get_unaligned_t(type, src), dst); \
> + __put_unaligned_t(type, 0, src); \
> + dst += sizeof(type); \
> + src += sizeof(type); \
> + len -= sizeof(type); \
> + } \
> +} while (0)
> +
> +static void memcpy_and_zero_src(void *dst, void *src, size_t len)
> +{
> + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) {
> + if (IS_ENABLED(CONFIG_64BIT))
> + MEMCPY_AND_ZERO_SRC(u64, dst, src, len);
> + MEMCPY_AND_ZERO_SRC(u32, dst, src, len);
> + MEMCPY_AND_ZERO_SRC(u16, dst, src, len);
> + }
> + MEMCPY_AND_ZERO_SRC(u8, dst, src, len);
> +}
> +
> +/**
> + * __cvdso_getrandom_data - Generic vDSO implementation of getrandom() syscall.
> + * @rng_info: Describes state of kernel RNG, memory shared with kernel.
> + * @buffer: Destination buffer to fill with random bytes.
> + * @len: Size of @buffer in bytes.
> + * @flags: Zero or more GRND_* flags.
> + * @opaque_state: Pointer to an opaque state area.
> + *
> + * This implements a "fast key erasure" RNG using ChaCha20, in the same way that the kernel's
> + * getrandom() syscall does. It periodically reseeds its key from the kernel's RNG, at the same
> + * schedule that the kernel's RNG is reseeded. If the kernel's RNG is not ready, then this always
> + * calls into the syscall.
> + *
> + * @opaque_state *must* be allocated using the vgetrandom_alloc() syscall. Unless external locking
> + * is used, one state must be allocated per thread, as it is not safe to call this function
> + * concurrently with the same @opaque_state. However, it is safe to call this using the same
> + * @opaque_state that is shared between main code and signal handling code, within the same thread.
> + *
> + * Returns the number of random bytes written to @buffer, or a negative value indicating an error.
> + */
> +static __always_inline ssize_t
> +__cvdso_getrandom_data(const struct vdso_rng_data *rng_info, void *buffer, size_t len,
> + unsigned int flags, void *opaque_state)
I don’t love this function signature. I generally think that, if
you’re going to have user code pass a pointer to kernel code, either
make the buffer have a well defined, constant size or pass a length.
As it stands, one cannot locally prove that user code that calls it is
memory-safe. In fact, any caller that has the misfortune of running
under CRIU is *not* memory safe if CRIU allows the vDSO to be
preserved. Ouch. (CRIU has some special code for this. I'm not 100%
clear on all the details.) One could maybe sort of get away with
treating the provided opaque_state as a completely opaque value and
not a pointer, but then the mechanism for allocating these states
should be adjusted accordingly.
One thing that occurs to me is that, if this thing were to be made
CRIU-safe, the buffer could have a magic number that changes any time
the data structure changes, and the vDSO could check, at the beginning
and end of the call, that the magic number is correct. Doing this
would require using a special VMA type instead of just a wipe-on-fork
mapping, which could plausibly be a good thing anyway. (Hmm, we don't
just want WIPEONFORK. We should probably also wipe on swap-out and,
more importantly, we should absolutely wipe on any sort of CRIU-style
checkpointing. Perhaps a special VMA would be a good thing for
multiple reasons.
> +{
> + ssize_t ret = min_t(size_t, INT_MAX & PAGE_MASK /* = MAX_RW_COUNT */, len);
> + struct vgetrandom_state *state = opaque_state;
> + size_t batch_len, nblocks, orig_len = len;
> + unsigned long current_generation;
> + void *orig_buffer = buffer;
> + u32 counter[2] = { 0 };
> + bool in_use, have_retried = false;
> +
> + /* The state must not straddle a page, since pages can be zeroed at any time. */
> + if (unlikely(((unsigned long)opaque_state & ~PAGE_MASK) + sizeof(*state) > PAGE_SIZE))
> + goto fallback_syscall;
This is weird. Either the provided pointer is valid or it isn’t.
Reasonable outcomes are a segfault if the pointer is bad or success
(or fallback if needed for some reason) if the pointer is good. Why
is there specific code to catch a specific sort of pointer screwup
here?
^ permalink raw reply
* Re: [PATCH v16 5/5] x86: vdso: Wire up getrandom() vDSO implementation
From: Randy Dunlap @ 2024-05-31 19:16 UTC (permalink / raw)
To: Jason A. Donenfeld, linux-kernel, patches, tglx
Cc: linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand,
Samuel Neves
In-Reply-To: <20240528122352.2485958-6-Jason@zx2c4.com>
On 5/28/24 5:19 AM, Jason A. Donenfeld wrote:
> +/**
> + * getrandom_syscall - Invoke the getrandom() syscall.
> + * @buffer: Destination buffer to fill with random bytes.
> + * @len: Size of @buffer in bytes.
> + * @flags: Zero or more GRND_* flags.
> + * Returns the number of random bytes written to @buffer, or a negative value indicating an error.
* Returns:
> + */
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
^ permalink raw reply
* Re: [PATCH v16 4/5] random: introduce generic vDSO getrandom() implementation
From: Randy Dunlap @ 2024-05-31 19:15 UTC (permalink / raw)
To: Jason A. Donenfeld, linux-kernel, patches, tglx
Cc: linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand
In-Reply-To: <20240528122352.2485958-5-Jason@zx2c4.com>
On 5/28/24 5:19 AM, Jason A. Donenfeld wrote:
> +/**
> + * __cvdso_getrandom_data - Generic vDSO implementation of getrandom() syscall.
> + * @rng_info: Describes state of kernel RNG, memory shared with kernel.
> + * @buffer: Destination buffer to fill with random bytes.
> + * @len: Size of @buffer in bytes.
> + * @flags: Zero or more GRND_* flags.
> + * @opaque_state: Pointer to an opaque state area.
> + *
> + * This implements a "fast key erasure" RNG using ChaCha20, in the same way that the kernel's
> + * getrandom() syscall does. It periodically reseeds its key from the kernel's RNG, at the same
> + * schedule that the kernel's RNG is reseeded. If the kernel's RNG is not ready, then this always
> + * calls into the syscall.
> + *
> + * @opaque_state *must* be allocated using the vgetrandom_alloc() syscall. Unless external locking
> + * is used, one state must be allocated per thread, as it is not safe to call this function
> + * concurrently with the same @opaque_state. However, it is safe to call this using the same
> + * @opaque_state that is shared between main code and signal handling code, within the same thread.
> + *
> + * Returns the number of random bytes written to @buffer, or a negative value indicating an error.
* Returns:
> + */
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
^ permalink raw reply
* Re: [PATCH v16 4/5] random: introduce generic vDSO getrandom() implementation
From: Randy Dunlap @ 2024-05-31 19:12 UTC (permalink / raw)
To: Jason A. Donenfeld, linux-kernel, patches, tglx
Cc: linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand
In-Reply-To: <20240528122352.2485958-5-Jason@zx2c4.com>
On 5/28/24 5:19 AM, Jason A. Donenfeld wrote:
> +/**
> + * type vdso_kernel_ulong - unsigned long type that matches kernel's unsigned long
s/type/typedef/ (for first "type" only)
> + *
> + * Data shared between userspace and the kernel must operate the same way in both 64-bit code and in
> + * 32-bit compat code, over the same potentially 64-bit kernel. This type represents the size of an
> + * unsigned long as used by kernel code. This isn't necessarily the same as an unsigned long as used
> + * by userspace, however.
> + *
> + * +-------------------+-------------------+------------------+-------------------+
> + * | 32-bit userspace | 32-bit userspace | 64-bit userspace | 64-bit userspace |
> + * | unsigned long | vdso_kernel_ulong | unsigned long | vdso_kernel_ulong |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + * | 32-bit kernel | ✓ same size | ✓ same size |
> + * | unsigned long | | |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + * | 64-bit kernel | ✘ different size! | ✓ same size | ✓ same size | ✓ same size |
> + * | unsigned long | | | | |
> + * +---------------+-------------------+-------------------+------------------+-------------------+
> + */
> +#ifdef CONFIG_64BIT
> +typedef u64 vdso_kernel_ulong;
> +#else
> +typedef u32 vdso_kernel_ulong;
> +#endif
--
#Randy
https://people.kernel.org/tglx/notes-about-netiquette
https://subspace.kernel.org/etiquette.html
^ permalink raw reply
* Re: [PATCHv7 bpf-next 0/9] uprobe: uretprobe speed up
From: Andrii Nakryiko @ 2024-05-31 17:52 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu
Cc: Oleg Nesterov, Jiri Olsa, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, linux-kernel, linux-trace-kernel, linux-api,
linux-man, x86, bpf, Song Liu, Yonghong Song, John Fastabend,
Peter Zijlstra, Thomas Gleixner, Borislav Petkov (AMD),
Ingo Molnar, Andy Lutomirski, Edgecombe, Rick P, Deepak Gupta
In-Reply-To: <20240523121149.575616-1-jolsa@kernel.org>
On Thu, May 23, 2024 at 5:11 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> hi,
> as part of the effort on speeding up the uprobes [0] coming with
> return uprobe optimization by using syscall instead of the trap
> on the uretprobe trampoline.
>
> The speed up depends on instruction type that uprobe is installed
> and depends on specific HW type, please check patch 1 for details.
>
> Patches 1-8 are based on bpf-next/master, but patch 2 and 3 are
> apply-able on linux-trace.git tree probes/for-next branch.
> Patch 9 is based on man-pages master.
>
> v7 changes:
> - fixes in man page [Alejandro Colomar]
> - fixed patch #1 fixes tag [Oleg]
>
> Also available at:
> https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
> uretprobe_syscall
>
> thanks,
> jirka
>
>
> Notes to check list items in Documentation/process/adding-syscalls.rst:
>
> - System Call Alternatives
> New syscall seems like the best way in here, because we need
> just to quickly enter kernel with no extra arguments processing,
> which we'd need to do if we decided to use another syscall.
>
> - Designing the API: Planning for Extension
> The uretprobe syscall is very specific and most likely won't be
> extended in the future.
>
> At the moment it does not take any arguments and even if it does
> in future, it's allowed to be called only from trampoline prepared
> by kernel, so there'll be no broken user.
>
> - Designing the API: Other Considerations
> N/A because uretprobe syscall does not return reference to kernel
> object.
>
> - Proposing the API
> Wiring up of the uretprobe system call is in separate change,
> selftests and man page changes are part of the patchset.
>
> - Generic System Call Implementation
> There's no CONFIG option for the new functionality because it
> keeps the same behaviour from the user POV.
>
> - x86 System Call Implementation
> It's 64-bit syscall only.
>
> - Compatibility System Calls (Generic)
> N/A uretprobe syscall has no arguments and is not supported
> for compat processes.
>
> - Compatibility System Calls (x86)
> N/A uretprobe syscall is not supported for compat processes.
>
> - System Calls Returning Elsewhere
> N/A.
>
> - Other Details
> N/A.
>
> - Testing
> Adding new bpf selftests and ran ltp on top of this change.
>
> - Man Page
> Attached.
>
> - Do not call System Calls in the Kernel
> N/A.
>
>
> [0] https://lore.kernel.org/bpf/ZeCXHKJ--iYYbmLj@krava/
> ---
> Jiri Olsa (8):
> x86/shstk: Make return uprobe work with shadow stack
> uprobe: Wire up uretprobe system call
> uprobe: Add uretprobe syscall to speed up return probe
> selftests/x86: Add return uprobe shadow stack test
> selftests/bpf: Add uretprobe syscall test for regs integrity
> selftests/bpf: Add uretprobe syscall test for regs changes
> selftests/bpf: Add uretprobe syscall call from user space test
> selftests/bpf: Add uretprobe shadow stack test
>
Masami, Steven,
It seems like the series is ready to go in. Are you planning to take
the first 4 patches through your linux-trace tree?
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> arch/x86/include/asm/shstk.h | 4 +
> arch/x86/kernel/shstk.c | 16 ++++
> arch/x86/kernel/uprobes.c | 124 ++++++++++++++++++++++++++++-
> include/linux/syscalls.h | 2 +
> include/linux/uprobes.h | 3 +
> include/uapi/asm-generic/unistd.h | 5 +-
> kernel/events/uprobes.c | 24 ++++--
> kernel/sys_ni.c | 2 +
> tools/include/linux/compiler.h | 4 +
> tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c | 123 ++++++++++++++++++++++++++++-
> tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c | 385 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> tools/testing/selftests/bpf/progs/uprobe_syscall.c | 15 ++++
> tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c | 17 ++++
> tools/testing/selftests/x86/test_shadow_stack.c | 145 ++++++++++++++++++++++++++++++++++
> 15 files changed, 860 insertions(+), 10 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_executed.c
>
> Jiri Olsa (1):
> man2: Add uretprobe syscall page
>
> man/man2/uretprobe.2 | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
> create mode 100644 man/man2/uretprobe.2
^ permalink raw reply
* Re: [PATCH v16 1/5] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Jann Horn @ 2024-05-31 13:00 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <Zlm-26QuqOSpXQg7@zx2c4.com>
On Fri, May 31, 2024 at 2:13 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Fri, May 31, 2024 at 12:48:58PM +0200, Jann Horn wrote:
> > On Tue, May 28, 2024 at 2:24 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > > c) If there's not enough memory to service a page fault, it's not fatal.
> > [...]
> > > @@ -5689,6 +5689,10 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
> > >
> > > lru_gen_exit_fault();
> > >
> > > + /* If the mapping is droppable, then errors due to OOM aren't fatal. */
> > > + if (vma->vm_flags & VM_DROPPABLE)
> > > + ret &= ~VM_FAULT_OOM;
> >
> > Can you remind me how this is supposed to work? If we get an OOM
> > error, and the error is not fatal, does that mean we'll just keep
> > hitting the same fault handler over and over again (until we happen to
> > have memory available again I guess)?
>
> Right, it'll just keep retrying. I agree this isn't great, which is why
> in the 2023 patchset, I had additional code to simply skip the faulting
> instruction, and then the userspace code would notice the inconsistency
> and fallback to the syscall. This worked pretty well. But it meant
> decoding the instruction and in general skipping instructions is weird,
> and that made this patchset very very contentious. Since the skipping
> behavior isn't actually required by the /security goals/ of this, I
> figured I'd just drop that. And maybe we can all revisit it together
> sometime down the line. But for now I'm hoping for something a little
> easier to swallow.
In that case, since we need to be able to populate this memory to make
forward progress, would it make sense to remove the parts of the patch
that treat the allocation as if it was allowed to silently fail (the
"__GFP_NOWARN | __GFP_NORETRY" and the "ret &= ~VM_FAULT_OOM")? I
think that would also simplify this a bit by making this type of
memory a little less special.
^ permalink raw reply
* Re: [PATCH v16 1/5] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-05-31 12:13 UTC (permalink / raw)
To: Jann Horn
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <CAG48ez0P3EDXC0uLLPjSjx3i6qB3fcdZbL2kYyuK6fZ_nJeN5w@mail.gmail.com>
On Fri, May 31, 2024 at 12:48:58PM +0200, Jann Horn wrote:
> On Tue, May 28, 2024 at 2:24 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > c) If there's not enough memory to service a page fault, it's not fatal.
> [...]
> > @@ -5689,6 +5689,10 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
> >
> > lru_gen_exit_fault();
> >
> > + /* If the mapping is droppable, then errors due to OOM aren't fatal. */
> > + if (vma->vm_flags & VM_DROPPABLE)
> > + ret &= ~VM_FAULT_OOM;
>
> Can you remind me how this is supposed to work? If we get an OOM
> error, and the error is not fatal, does that mean we'll just keep
> hitting the same fault handler over and over again (until we happen to
> have memory available again I guess)?
Right, it'll just keep retrying. I agree this isn't great, which is why
in the 2023 patchset, I had additional code to simply skip the faulting
instruction, and then the userspace code would notice the inconsistency
and fallback to the syscall. This worked pretty well. But it meant
decoding the instruction and in general skipping instructions is weird,
and that made this patchset very very contentious. Since the skipping
behavior isn't actually required by the /security goals/ of this, I
figured I'd just drop that. And maybe we can all revisit it together
sometime down the line. But for now I'm hoping for something a little
easier to swallow.
Jason
^ permalink raw reply
* Re: [PATCH v16 1/5] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Jann Horn @ 2024-05-31 10:48 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <20240528122352.2485958-2-Jason@zx2c4.com>
On Tue, May 28, 2024 at 2:24 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> c) If there's not enough memory to service a page fault, it's not fatal.
[...]
> @@ -5689,6 +5689,10 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>
> lru_gen_exit_fault();
>
> + /* If the mapping is droppable, then errors due to OOM aren't fatal. */
> + if (vma->vm_flags & VM_DROPPABLE)
> + ret &= ~VM_FAULT_OOM;
Can you remind me how this is supposed to work? If we get an OOM
error, and the error is not fatal, does that mean we'll just keep
hitting the same fault handler over and over again (until we happen to
have memory available again I guess)?
Or is there something in this series that somehow redirects userspace
execution to getrandom() in that case?
> +
> if (flags & FAULT_FLAG_USER) {
> mem_cgroup_exit_user_fault();
> /*
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Christian Brauner @ 2024-05-31 10:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, Aleksa Sarai, Alexander Viro, Chuck Lever, Jeff Layton,
Amir Goldstein, Alexander Aring, linux-fsdevel, linux-nfs,
linux-kernel, linux-api
In-Reply-To: <ZlmG1Rss6gTgbSVT@infradead.org>
> I don't understand what you mean. If we hand out file handles with
I don't think adding another highly privileged interface in opposition
to exposing the mount id is warranted. The new mount id exposure patch
is a performance improvement for existing use-cases to let userspace
avoid additional system calls. Because as Aleksa showed in the commit
message there's ways to make this race-free right now. The patch hasn't
gained any Acks from Amir or Jeff so it's obviously not going to get
merged. So really, I don't want another interface that goes from
arbitrary superblock to file hidden behind yet another global capability
check.
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Christoph Hellwig @ 2024-05-31 8:14 UTC (permalink / raw)
To: Christian Brauner
Cc: Christoph Hellwig, Jan Kara, Aleksa Sarai, Alexander Viro,
Chuck Lever, Jeff Layton, Amir Goldstein, Alexander Aring,
linux-fsdevel, linux-nfs, linux-kernel, linux-api
In-Reply-To: <20240529-marzipan-verspannungen-48b760c2f66b@brauner>
On Wed, May 29, 2024 at 09:40:01AM +0200, Christian Brauner wrote:
> Yeah, that's exactly what I figured and no that's not something we
> should do.
>
> Not just can have a really large number of superblocks if you have mount
> namespaces and large container workloads that interface also needs to be
> highly privileged.
Again, that would be the most trivial POC. We can easily do hash.
> Plus, you do have filesystems like btrfs that can be mounted multiple
> times with the same uuid.
Which doesn't matter. Just like for NFS file handles the fs identifier
identifier plus the file part of the file handle need to be unique.
> And in general users will still need to be able to legitimately use a
> mount fd and not care about the handle type used with it.
I don't understand what you mean. If we hand out file handles with
fsid that of course needs to be keyed off a new flag for both
name_to_handle and open_by_hnalde that makes them not interchangable
to handles generated without that flag.
^ permalink raw reply
* Re: [PATCH v16 2/5] random: add vgetrandom_alloc() syscall
From: Eric Biggers @ 2024-05-31 3:59 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand
In-Reply-To: <20240528122352.2485958-3-Jason@zx2c4.com>
On Tue, May 28, 2024 at 02:19:51PM +0200, Jason A. Donenfeld wrote:
> +/**
> + * sys_vgetrandom_alloc - Allocate opaque states for use with vDSO getrandom().
> + *
> + * @num: On input, a pointer to a suggested hint of how many states to
> + * allocate, and on return the number of states actually allocated.
> + *
> + * @size_per_each: On input, must be zero. On return, the size of each state allocated,
> + * so that the caller can split up the returned allocation into
> + * individual states.
> + *
> + * @addr: Reserved, must be zero.
> + *
> + * @flags: Reserved, must be zero.
> + *
> + * The getrandom() vDSO function in userspace requires an opaque state, which
> + * this function allocates by mapping a certain number of special pages into
> + * the calling process. It takes a hint as to the number of opaque states
> + * desired, and provides the caller with the number of opaque states actually
> + * allocated, the size of each one in bytes, and the address of the first
> + * state, which may be split up into @num states of @size_per_each bytes each,
> + * by adding @size_per_each to the returned first state @num times, while
> + * ensuring that no single state straddles a page boundary.
> + *
> + * Returns the address of the first state in the allocation on success, or a
> + * negative error value on failure.
> + *
> + * The returned address of the first state may be passed to munmap(2) with a
> + * length of `(size_t)num * (size_t)size_per_each`, in order to deallocate the
> + * memory, after which it is invalid to pass it to vDSO getrandom().
Wouldn't a munmap with '(size_t)num * (size_t)size_per_each' be potentially too
short, due to how the allocation is sized such that states don't cross page
boundaries?
- Eric
^ permalink raw reply
* Re: [PATCH v16 5/5] x86: vdso: Wire up getrandom() vDSO implementation
From: Eric Biggers @ 2024-05-31 3:38 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, Samuel Neves
In-Reply-To: <20240528122352.2485958-6-Jason@zx2c4.com>
On Tue, May 28, 2024 at 02:19:54PM +0200, Jason A. Donenfeld wrote:
> diff --git a/arch/x86/entry/vdso/vgetrandom-chacha.S b/arch/x86/entry/vdso/vgetrandom-chacha.S
> new file mode 100644
> index 000000000000..d79e2bd97598
> --- /dev/null
> +++ b/arch/x86/entry/vdso/vgetrandom-chacha.S
> @@ -0,0 +1,178 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#include <linux/linkage.h>
> +#include <asm/frame.h>
> +
> +.section .rodata, "a"
> +.align 16
> +CONSTANTS: .octa 0x6b20657479622d323320646e61707865
> +.text
> +
> +/*
> + * Very basic SSE2 implementation of ChaCha20. Produces a given positive number
> + * of blocks of output with a nonce of 0, taking an input key and 8-byte
> + * counter. Importantly does not spill to the stack. Its arguments are:
> + *
> + * rdi: output bytes
> + * rsi: 32-byte key input
> + * rdx: 8-byte counter input/output
> + * rcx: number of 64-byte blocks to write to output
> + */
> +SYM_FUNC_START(__arch_chacha20_blocks_nostack)
> +
> +.set output, %rdi
> +.set key, %rsi
> +.set counter, %rdx
> +.set nblocks, %rcx
> +.set i, %al
> +/* xmm registers are *not* callee-save. */
> +.set state0, %xmm0
> +.set state1, %xmm1
> +.set state2, %xmm2
> +.set state3, %xmm3
> +.set copy0, %xmm4
> +.set copy1, %xmm5
> +.set copy2, %xmm6
> +.set copy3, %xmm7
> +.set temp, %xmm8
> +.set one, %xmm9
An "interesting" x86_64 quirk: in SSE instructions, registers xmm0-xmm7 take
fewer bytes to encode than xmm8-xmm15.
Since 'temp' is used frequently, moving it into the lower range (and moving one
of the 'copy' registers, which isn't used as frequently, into the higher range)
decreases the code size of __arch_chacha20_blocks_nostack() by 5%.
- Eric
^ permalink raw reply
* Re: [PATCH v16 3/5] arch: allocate vgetrandom_alloc() syscall number
From: Eric Biggers @ 2024-05-31 2:26 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Geert Uytterhoeven, linux-kernel, patches, tglx, linux-crypto,
linux-api, x86, Greg Kroah-Hartman, Adhemerval Zanella Netto,
Carlos O'Donell, Florian Weimer, Arnd Bergmann, Jann Horn,
Christian Brauner, David Hildenbrand
In-Reply-To: <CAHmME9qZ2Xm68HQqfOmasWRoaW4xfAFAiLfw-pFbhq2i0rHz=w@mail.gmail.com>
On Tue, May 28, 2024 at 03:28:58PM +0200, Jason A. Donenfeld wrote:
> On Tue, May 28, 2024 at 3:10 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > On Tue, May 28, 2024 at 03:08:00PM +0200, Geert Uytterhoeven wrote:
> > > Hi Jason,
> > >
> > > On Tue, May 28, 2024 at 2:24 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> > > > Add vgetrandom_alloc() as syscall 462 (or 572 on alpha) by adding it to
> > > > all of the various syscall.tbl and unistd.h files.
> > > >
> > > > Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> > > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > >
> > > As of commit ff388fe5c481d39c ("mseal: wire up mseal syscall") in
> > > v6.10-rc1, 462 is already taken.
> > >
> > > v17 ++ ;-)
> >
> > Oy! Thanks. I should have thought to rebase on rc1 anyway before posting
> > this.
>
> Sorted in https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git/log/?h=vdso
> for the time being.
>
Please also get in the habit of using the --base option to git format-patch, so
that it's actually possible to apply patches without guessing the base commit.
Thanks,
- Eric
^ permalink raw reply
* Re: [PATCHv6 bpf-next 1/9] x86/shstk: Make return uprobe work with shadow stack
From: Edgecombe, Rick P @ 2024-05-30 23:04 UTC (permalink / raw)
To: jolsa@kernel.org, mhiramat@kernel.org, rostedt@goodmis.org,
ast@kernel.org, andrii@kernel.org, oleg@redhat.com,
daniel@iogearbox.net
Cc: debug@rivosinc.com, luto@kernel.org, bp@alien8.de, yhs@fb.com,
songliubraving@fb.com, linux-api@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, john.fastabend@gmail.com,
tglx@linutronix.de, mingo@redhat.com,
linux-trace-kernel@vger.kernel.org, bpf@vger.kernel.org,
linux-man@vger.kernel.org, peterz@infradead.org
In-Reply-To: <20240521104825.1060966-2-jolsa@kernel.org>
On Tue, 2024-05-21 at 12:48 +0200, Jiri Olsa wrote:
> Currently the application with enabled shadow stack will crash
> if it sets up return uprobe. The reason is the uretprobe kernel
> code changes the user space task's stack, but does not update
> shadow stack accordingly.
>
> Adding new functions to update values on shadow stack and using
> them in uprobe code to keep shadow stack in sync with uretprobe
> changes to user stack.
>
> Fixes: 8b1c23543436 ("x86/shstk: Add return uprobe support")
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
Acked-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
^ permalink raw reply
* Re: [PATCH v5 1/3] VT: Use macros to define ioctls
From: Al Viro @ 2024-05-29 8:44 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg Kroah-Hartman, Alexey Gladkov, LKML, kbd, linux-api,
linux-fbdev, linux-serial, Arnd Bergmann
In-Reply-To: <0da9785e-ba44-4718-9d08-4e96c1ba7ab2@kernel.org>
On Wed, May 29, 2024 at 09:29:23AM +0200, Jiri Slaby wrote:
> On 18. 04. 24, 8:18, Greg Kroah-Hartman wrote:
> > On Wed, Apr 17, 2024 at 07:37:35PM +0200, Alexey Gladkov wrote:
> > > All other headers use _IOC() macros to describe ioctls for a long time
> > > now. This header is stuck in the last century.
> > >
> > > Simply use the _IO() macro. No other changes.
> > >
> > > Signed-off-by: Alexey Gladkov <legion@kernel.org>
> > > ---
> > > include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
> > > 1 file changed, 49 insertions(+), 47 deletions(-)
> >
> > This is a nice cleanup, thanks for doing it, I'll just take this one
> > change now if you don't object.
>
> Unfortunately, _IOC_NONE is 1 on some archs as noted by Arnd, and this
> commit changed the kd ioctl values in there which broke stuff as noted by
> Al.
>
> We either:
> * use _IOC(0, X, Y) in here, instead of _IO(X, Y), or
> * define KDIOC(X) as _IOC(0, KD_IOCTL_BASE, X), or
> * revert the commit which landed to -rc1 already.
Revert, IMO. If nothing else, _IO... macros are there to
* document the copyin/copyout direction
* make sure that argument size mismatches between the kernel
and userland get caught
Turning 16bit hex constants into that won't serve any purpose other
than preventing such patches in the future; adding a comment would
deal with that just fine.
BTW, the original odd choice for _IOC_NONE appears to have been motivated
by avoiding conflicts with legacy ioctl numbers: in CSRG repo there's
this:
commit 02cbc6a653b48bb4ec6052cbe6b3979530011c46
Author: Sam Leffler <sam@ucbvax.Berkeley.EDU>
Date: Mon Aug 2 02:22:17 1982 -0800
new ioctl's
...
+/*
+ * Ioctl's have the command encoded in the lower word,
+ * and the size of any in or out parameters in the upper
+ * word. The high 2 bits of the upper word are used
+ * to encode the in/out status of the parameter; for now
+ * we restrict parameters to at most 128 bytes.
+ */
+#define IOCPARM_MASK 0x7f /* parameters must be < 128 bytes */
+#define IOC_VOID 0x20000000 /* no parameters */
+#define IOC_OUT 0x40000000 /* copy out parameters */
+#define IOC_IN 0x80000000 /* copy in parameters */
+#define IOC_INOUT (IOC_IN|IOC_OUT)
+/* the 0x20000000 is so we can distinguish new ioctl's from old */
+#define _IO(x,y) (IOC_VOID|('x'<<8)|y)
+#define _IOR(x,y,t) (IOC_OUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+#define _IOW(x,y,t) (IOC_IN|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
+/* this should be _IORW, but stdio got there first */
+#define _IOWR(x,y,t) (IOC_INOUT|((sizeof(t)&IOCPARM_MASK)<<16)|('x'<<8)|y)
FWIW, the upper limit on parameter size went up from 128 bytes to 8Kb in
commit 856ed9ecd2795280feed42baf3889ac7c7f9a26d
Author: Mike Karels <karels@ucbvax.Berkeley.EDU>
Date: Thu Feb 19 22:16:28 1987 -0800
larger ioctl's (for disklabels)
Wonder what got us (in sparc port?) to lift it from 8Kb to 16Kb...
In our historical tree that has happened in
commit 1c29224f169362b671a4bef4cd864464ef810d42
Author: Pete Zaitcev <zaitcev@redhat.com>
Date: Sun Mar 2 08:45:12 2003 -0800
[SPARC32/64]: Expand ioctl size field in backwards-compatible way.
No explanations there...
^ permalink raw reply
* Re: [PATCH v5 1/3] VT: Use macros to define ioctls
From: Arnd Bergmann @ 2024-05-29 7:44 UTC (permalink / raw)
To: Jiri Slaby, Greg Kroah-Hartman, Alexey Gladkov
Cc: LKML, kbd, linux-api, linux-fbdev, linux-serial, Alexander Viro
In-Reply-To: <0da9785e-ba44-4718-9d08-4e96c1ba7ab2@kernel.org>
On Wed, May 29, 2024, at 09:29, Jiri Slaby wrote:
> On 18. 04. 24, 8:18, Greg Kroah-Hartman wrote:
>>
>> This is a nice cleanup, thanks for doing it, I'll just take this one
>> change now if you don't object.
>
> Unfortunately, _IOC_NONE is 1 on some archs as noted by Arnd, and this
> commit changed the kd ioctl values in there which broke stuff as noted
> by Al.
>
> We either:
> * use _IOC(0, X, Y) in here, instead of _IO(X, Y), or
> * define KDIOC(X) as _IOC(0, KD_IOCTL_BASE, X), or
> * revert the commit which landed to -rc1 already.
I would prefer a simple revert, as the other options may
end up more confusing. Another option might be a new
global macro, if we then go an convert all plain ioctl
command numbers to that.
Arnd
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Christian Brauner @ 2024-05-29 7:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jan Kara, Aleksa Sarai, Alexander Viro, Chuck Lever, Jeff Layton,
Amir Goldstein, Alexander Aring, linux-fsdevel, linux-nfs,
linux-kernel, linux-api
In-Reply-To: <ZlXaj9Qv0bm9PAjX@infradead.org>
On Tue, May 28, 2024 at 06:22:23AM -0700, Christoph Hellwig wrote:
> On Tue, May 28, 2024 at 02:04:16PM +0200, Christian Brauner wrote:
> > Can you please explain how opening an fd based on a handle returned from
> > name_to_handle_at() and not using a mount file descriptor for
> > open_by_handle_at() would work?
>
> Same as NFS file handles:
>
> name_to_handle_at returns a handle that includes a file system
> identifier.
>
> open_by_handle_at looks up the superblock based on that identifier.
>
> For the identifier I could imagin three choices:
>
> 1) use the fsid as returned in statfs and returned by fsnotify.
> The downside is that it is "only" 64-bit. The upside is that
> we have a lot of plumbing for it
> 2) fixed 128-bit identifier to provide more entropy
> 3) a variable length identifier, which is more similar to NFS,
> but also a lot more complicated
>
> We'd need a global lookup structure to find the sb by id. The simplest
> one would be a simple linear loop over super_blocks which isn't terribly
> efficient, but probably better than whatever userspace is doing to
> find a mount fd right now.
>
> Let me cook up a simple prototype for 1) as it shouldn't be more than
> a few hundred lines of code.
Yeah, that's exactly what I figured and no that's not something we
should do.
Not just can have a really large number of superblocks if you have mount
namespaces and large container workloads that interface also needs to be
highly privileged.
Plus, you do have filesystems like btrfs that can be mounted multiple
times with the same uuid.
And in general users will still need to be able to legitimately use a
mount fd and not care about the handle type used with it.
^ permalink raw reply
* Re: [PATCH v5 1/3] VT: Use macros to define ioctls
From: Jiri Slaby @ 2024-05-29 7:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Alexey Gladkov
Cc: LKML, kbd, linux-api, linux-fbdev, linux-serial, Al Viro,
Arnd Bergmann
In-Reply-To: <2024041836-most-ablaze-f417@gregkh>
On 18. 04. 24, 8:18, Greg Kroah-Hartman wrote:
> On Wed, Apr 17, 2024 at 07:37:35PM +0200, Alexey Gladkov wrote:
>> All other headers use _IOC() macros to describe ioctls for a long time
>> now. This header is stuck in the last century.
>>
>> Simply use the _IO() macro. No other changes.
>>
>> Signed-off-by: Alexey Gladkov <legion@kernel.org>
>> ---
>> include/uapi/linux/kd.h | 96 +++++++++++++++++++++--------------------
>> 1 file changed, 49 insertions(+), 47 deletions(-)
>
> This is a nice cleanup, thanks for doing it, I'll just take this one
> change now if you don't object.
Unfortunately, _IOC_NONE is 1 on some archs as noted by Arnd, and this
commit changed the kd ioctl values in there which broke stuff as noted
by Al.
We either:
* use _IOC(0, X, Y) in here, instead of _IO(X, Y), or
* define KDIOC(X) as _IOC(0, KD_IOCTL_BASE, X), or
* revert the commit which landed to -rc1 already.
thanks,
--
js
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Amir Goldstein @ 2024-05-29 7:23 UTC (permalink / raw)
To: hch@infradead.org
Cc: Dave Chinner, Jan Kara, Trond Myklebust, chuck.lever@oracle.com,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
brauner@kernel.org, linux-kernel@vger.kernel.org,
alex.aring@gmail.com, cyphar@cyphar.com, viro@zeniv.linux.org.uk,
jlayton@kernel.org, linux-nfs@vger.kernel.org
In-Reply-To: <ZlbKEr15IXO2jxXd@infradead.org>
On Wed, May 29, 2024 at 9:24 AM hch@infradead.org <hch@infradead.org> wrote:
>
> On Wed, May 29, 2024 at 09:25:49AM +1000, Dave Chinner wrote:
> > But no-one has bothered to reply or acknowledge my comments so I'll
> > point them out again and repeat: Filehandles generated by
> > the kernel for unprivileged use *must* be self describing and self
> > validating
Very nice requirement for a very strong feature,
which is far out of scope for the proposed patch IMO.
What is "generated by the kernel for unprivileged use"?
name_to_handle_at() is unprivileged and for example, fanotify unprivileged
users can use it to compare a marked (i.e. watched) object with an
object that is the subject of an event.
This does not break any security model.
> > as the kernel must be able to detect and prevent
> > unprivelged users from generating custom filehandles that can be
> > used to access files outside the restricted scope of their
> > container.
Emphasis on "that can be used to access files".
The patch in this thread to get a unique 64bit mntid does not make any
difference wrt to the concern above, so I am having a hard time
understand what the fuss is about.
>
> We must not generate file handle for unprivileged use at all, as they
> bypass all the path based access controls.
>
Again, how is "generate file handle for unprivileged use" related to
the patch in question.
The simple solution to the race that Aleksa is trying to prevent,
as was mentioned several times in this thread, is to allow
name_to_handle_at() on an empty path, e.g.:
fd = openat(.., O_PATH); fstatat(fd,..); name_to_handle_at(fd,..);
Aleksa prefers the unique mntid solution to save 2 syscalls.
Would any of the objections here been called for letting
name_to_handle_at() take an empty path?
I would like to offer a different solution (also may have been
mentioned before).
I always disliked the fact that mount_id and mount_fd arguments of
name_to_handle_at()/open_by_handle_at() are not symmetric, when
at first glance they appear to be symmetric as the handle argument.
What if we can make them symmetric and save the openat() syscall?
int path_fd;
int ret = name_to_handle_at(dirfd, path, handle, &path_fd,
AT_HADNLE_PATH_FD);
and then kernel returns an O_PATH fd to the path object
in addition to the file handle (requires same privilege as
encoding the fh).
Userspace can use path_fd to query unique mntid, fsid, uuid
or whatever it needs to know in order to make sure that a
later call in the future to open_by_handle_at() will use
a mount_fd from the same filesystem/mount whatever,
whether in the same boot or after reboot.
If we are doing this, it also makes sense to allow mount_fd argument
to open_by_handle_at() to accept O_PATH fd, but that makes
sense anyway, because mount_fd is essentially a reference to
a path.
Thanks,
Amir.
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Christoph Hellwig @ 2024-05-29 6:34 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Christoph Hellwig, Christian Brauner, Jan Kara, Aleksa Sarai,
Alexander Viro, Chuck Lever, Jeff Layton, Amir Goldstein,
Alexander Aring, linux-fsdevel, linux-nfs, linux-kernel,
linux-api
In-Reply-To: <CAJfpegvznUGTYxxTzB5QQHWtNrCfSkWvGscacfZ67Gn+6XoD8w@mail.gmail.com>
On Tue, May 28, 2024 at 03:28:18PM +0200, Miklos Szeredi wrote:
> > open_by_handle_at looks up the superblock based on that identifier.
>
> The open file needs a specific mount, holding the superblock is not
> sufficient.
A strut file needs a vfsmount, yes. And it better be reachable by
the calling process. And maybe an optional restriction to a specific
mount by the caller might be useful, but I can't see how it is
required.
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: hch @ 2024-05-29 6:24 UTC (permalink / raw)
To: Dave Chinner
Cc: hch@infradead.org, Jan Kara, Trond Myklebust,
chuck.lever@oracle.com, linux-api@vger.kernel.org,
linux-fsdevel@vger.kernel.org, brauner@kernel.org,
linux-kernel@vger.kernel.org, alex.aring@gmail.com,
cyphar@cyphar.com, viro@zeniv.linux.org.uk, jlayton@kernel.org,
amir73il@gmail.com, linux-nfs@vger.kernel.org
In-Reply-To: <ZlZn/fcphsx8u/Ph@dread.disaster.area>
On Wed, May 29, 2024 at 09:25:49AM +1000, Dave Chinner wrote:
> But no-one has bothered to reply or acknowledge my comments so I'll
> point them out again and repeat: Filehandles generated by
> the kernel for unprivileged use *must* be self describing and self
> validating as the kernel must be able to detect and prevent
> unprivelged users from generating custom filehandles that can be
> used to access files outside the restricted scope of their
> container.
We must not generate file handle for unprivileged use at all, as they
bypass all the path based access controls.
^ permalink raw reply
* Re: [PATCH RFC v2] fhandle: expose u64 mount id to name_to_handle_at(2)
From: Dave Chinner @ 2024-05-28 23:25 UTC (permalink / raw)
To: hch@infradead.org
Cc: Jan Kara, Trond Myklebust, chuck.lever@oracle.com,
linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org,
brauner@kernel.org, linux-kernel@vger.kernel.org,
alex.aring@gmail.com, cyphar@cyphar.com, viro@zeniv.linux.org.uk,
jlayton@kernel.org, amir73il@gmail.com, linux-nfs@vger.kernel.org
In-Reply-To: <ZlW4a6Zdt9SPTt80@infradead.org>
On Tue, May 28, 2024 at 03:56:43AM -0700, hch@infradead.org wrote:
> On Tue, May 28, 2024 at 12:11:52PM +0200, Jan Kara wrote:
> > So some fanotify users may use open_by_handle_at() and name_to_handle_at()
> > but we specifically designed fanotify to not depend on this mount id
> > feature of the API (because it wasn't really usable couple of years ago
> > when we were designing this with Amir). fanotify returns fsid + fhandle in
> > its events and userspace is expected to build a mapping of fsid ->
> > "whatever it needs to identify a filesystem" when placing fanotify marks.
> > If it wants to open file / directory where events happened, then this
> > usually means keeping fsid -> "some open fd on fs" mapping so that it can
> > then use open_by_handle_at() for opening.
>
> Which seems like another argument for my version of the handles to
> include the fsid. Although IIRC the fanotify fsid is only 64 bits which
> isn't really good entropy, so we might have to rev that as well.
I'm in agreement with Christoph that the filehandle needs to contain
the restricted scope information internally. I said that in response
to an earlier version of the patch here:
https://lore.kernel.org/linux-fsdevel/ZlPOd0p7AUn7JqLu@dread.disaster.area/
"If filehandles are going to be restricted to a specific container
(e.g. a single mount) so that less permissions are needed to open
the filehandle, then the filehandle itself needs to encode those
restrictions. Decoding the filehandle needs to ensure that the
opening context has permission to access whatever the filehandle
points to in a restricted environment. This will prevent existing
persistent, global filehandles from being used as restricted
filehandles and vice versa."
But no-one has bothered to reply or acknowledge my comments so I'll
point them out again and repeat: Filehandles generated by
the kernel for unprivileged use *must* be self describing and self
validating as the kernel must be able to detect and prevent
unprivelged users from generating custom filehandles that can be
used to access files outside the restricted scope of their
container.
-Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply
* Re: [PATCH v16 1/5] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Jason A. Donenfeld @ 2024-05-28 20:51 UTC (permalink / raw)
To: Frank van der Linden
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <CAPTztWbDvrrCQH49q0GTk54--Hzh1V_A7KRw4-z38eGQupPKJg@mail.gmail.com>
On Tue, May 28, 2024 at 01:41:50PM -0700, Frank van der Linden wrote:
> On Tue, May 28, 2024 at 5:24 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > The vDSO getrandom() implementation works with a buffer allocated with a
> > new system call that has certain requirements:
> >
> > - It shouldn't be written to core dumps.
> > * Easy: VM_DONTDUMP.
> > - It should be zeroed on fork.
> > * Easy: VM_WIPEONFORK.
> >
> > - It shouldn't be written to swap.
> > * Uh-oh: mlock is rlimited.
> > * Uh-oh: mlock isn't inherited by forks.
> >
> > - It shouldn't reserve actual memory, but it also shouldn't crash when
> > page faulting in memory if none is available
> > * Uh-oh: MAP_NORESERVE respects vm.overcommit_memory=2.
> > * Uh-oh: VM_NORESERVE means segfaults.
> >
> > It turns out that the vDSO getrandom() function has three really nice
> > characteristics that we can exploit to solve this problem:
> >
> > 1) Due to being wiped during fork(), the vDSO code is already robust to
> > having the contents of the pages it reads zeroed out midway through
> > the function's execution.
> >
> > 2) In the absolute worst case of whatever contingency we're coding for,
> > we have the option to fallback to the getrandom() syscall, and
> > everything is fine.
> >
> > 3) The buffers the function uses are only ever useful for a maximum of
> > 60 seconds -- a sort of cache, rather than a long term allocation.
> >
> > These characteristics mean that we can introduce VM_DROPPABLE, which
> > has the following semantics:
> >
> > a) It never is written out to swap.
> > b) Under memory pressure, mm can just drop the pages (so that they're
> > zero when read back again).
> > c) If there's not enough memory to service a page fault, it's not fatal.
> > d) It is inherited by fork.
> > e) It doesn't count against the mlock budget, since nothing is locked.
> >
> > This is fairly simple to implement, with the one snag that we have to
> > use 64-bit VM_* flags, but this shouldn't be a problem, since the only
> > consumers will probably be 64-bit anyway.
> >
> > This way, allocations used by vDSO getrandom() can use:
> >
> > VM_DROPPABLE | VM_DONTDUMP | VM_WIPEONFORK | VM_NORESERVE
> >
> > And there will be no problem with OOMing, crashing on overcommitment,
> > using memory when not in use, not wiping on fork(), coredumps, or
> > writing out to swap.
> >
> > Cc: linux-mm@kvack.org
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> > fs/proc/task_mmu.c | 3 +++
> > include/linux/mm.h | 8 ++++++++
> > include/trace/events/mmflags.h | 7 +++++++
> > mm/Kconfig | 3 +++
> > mm/memory.c | 4 ++++
> > mm/mempolicy.c | 3 +++
> > mm/mprotect.c | 2 +-
> > mm/rmap.c | 8 +++++---
> > 8 files changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> > index e5a5f015ff03..b5a59e57bde1 100644
> > --- a/fs/proc/task_mmu.c
> > +++ b/fs/proc/task_mmu.c
> > @@ -706,6 +706,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> > #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
> > #ifdef CONFIG_X86_USER_SHADOW_STACK
> > [ilog2(VM_SHADOW_STACK)] = "ss",
> > +#endif
> > +#ifdef CONFIG_NEED_VM_DROPPABLE
> > + [ilog2(VM_DROPPABLE)] = "dp",
> > #endif
> > };
> > size_t i;
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 9849dfda44d4..5978cb4cc21c 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -321,12 +321,14 @@ extern unsigned int kobjsize(const void *objp);
> > #define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit architectures */
> > #define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit architectures */
> > #define VM_HIGH_ARCH_BIT_5 37 /* bit only usable on 64-bit architectures */
> > +#define VM_HIGH_ARCH_BIT_6 38 /* bit only usable on 64-bit architectures */
> > #define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
> > #define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
> > #define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
> > #define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
> > #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
> > #define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
> > +#define VM_HIGH_ARCH_6 BIT(VM_HIGH_ARCH_BIT_6)
> > #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
> >
> > #ifdef CONFIG_ARCH_HAS_PKEYS
> > @@ -357,6 +359,12 @@ extern unsigned int kobjsize(const void *objp);
> > # define VM_SHADOW_STACK VM_NONE
> > #endif
> >
> > +#ifdef CONFIG_NEED_VM_DROPPABLE
> > +# define VM_DROPPABLE VM_HIGH_ARCH_6
> > +#else
> > +# define VM_DROPPABLE VM_NONE
> > +#endif
> > +
> > #if defined(CONFIG_X86)
> > # define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
> > #elif defined(CONFIG_PPC)
> > diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> > index e46d6e82765e..fab7848df50a 100644
> > --- a/include/trace/events/mmflags.h
> > +++ b/include/trace/events/mmflags.h
> > @@ -165,6 +165,12 @@ IF_HAVE_PG_ARCH_X(arch_3)
> > # define IF_HAVE_UFFD_MINOR(flag, name)
> > #endif
> >
> > +#ifdef CONFIG_NEED_VM_DROPPABLE
> > +# define IF_HAVE_VM_DROPPABLE(flag, name) {flag, name},
> > +#else
> > +# define IF_HAVE_VM_DROPPABLE(flag, name)
> > +#endif
> > +
> > #define __def_vmaflag_names \
> > {VM_READ, "read" }, \
> > {VM_WRITE, "write" }, \
> > @@ -197,6 +203,7 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty" ) \
> > {VM_MIXEDMAP, "mixedmap" }, \
> > {VM_HUGEPAGE, "hugepage" }, \
> > {VM_NOHUGEPAGE, "nohugepage" }, \
> > +IF_HAVE_VM_DROPPABLE(VM_DROPPABLE, "droppable" ) \
> > {VM_MERGEABLE, "mergeable" } \
> >
> > #define show_vma_flags(flags) \
> > diff --git a/mm/Kconfig b/mm/Kconfig
> > index b4cb45255a54..6cd65ea4b3ad 100644
> > --- a/mm/Kconfig
> > +++ b/mm/Kconfig
> > @@ -1056,6 +1056,9 @@ config ARCH_USES_HIGH_VMA_FLAGS
> > bool
> > config ARCH_HAS_PKEYS
> > bool
> > +config NEED_VM_DROPPABLE
> > + select ARCH_USES_HIGH_VMA_FLAGS
> > + bool
> >
> > config ARCH_USES_PG_ARCH_X
> > bool
> > diff --git a/mm/memory.c b/mm/memory.c
> > index b5453b86ec4b..57b03fc73159 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -5689,6 +5689,10 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
> >
> > lru_gen_exit_fault();
> >
> > + /* If the mapping is droppable, then errors due to OOM aren't fatal. */
> > + if (vma->vm_flags & VM_DROPPABLE)
> > + ret &= ~VM_FAULT_OOM;
> > +
> > if (flags & FAULT_FLAG_USER) {
> > mem_cgroup_exit_user_fault();
> > /*
> > diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> > index aec756ae5637..a66289f1d931 100644
> > --- a/mm/mempolicy.c
> > +++ b/mm/mempolicy.c
> > @@ -2300,6 +2300,9 @@ struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct
> > pgoff_t ilx;
> > struct page *page;
> >
> > + if (vma->vm_flags & VM_DROPPABLE)
> > + gfp |= __GFP_NOWARN | __GFP_NORETRY;
> > +
> > pol = get_vma_policy(vma, addr, order, &ilx);
> > page = alloc_pages_mpol_noprof(gfp | __GFP_COMP, order,
> > pol, ilx, numa_node_id());
> > diff --git a/mm/mprotect.c b/mm/mprotect.c
> > index 94878c39ee32..88ff3ecc08a1 100644
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -622,7 +622,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
> > may_expand_vm(mm, oldflags, nrpages))
> > return -ENOMEM;
> > if (!(oldflags & (VM_ACCOUNT|VM_WRITE|VM_HUGETLB|
> > - VM_SHARED|VM_NORESERVE))) {
> > + VM_SHARED|VM_NORESERVE|VM_DROPPABLE))) {
> > charged = nrpages;
> > if (security_vm_enough_memory_mm(mm, charged))
> > return -ENOMEM;
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > index e8fc5ecb59b2..d873a3f06506 100644
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -1397,7 +1397,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
> > VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
> > VM_BUG_ON_VMA(address < vma->vm_start ||
> > address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
> > - __folio_set_swapbacked(folio);
> > + if (!(vma->vm_flags & VM_DROPPABLE))
> > + __folio_set_swapbacked(folio);
> > __folio_set_anon(folio, vma, address, true);
> >
> > if (likely(!folio_test_large(folio))) {
> > @@ -1841,7 +1842,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> > * plus the rmap(s) (dropped by discard:).
> > */
> > if (ref_count == 1 + map_count &&
> > - !folio_test_dirty(folio)) {
> > + (!folio_test_dirty(folio) || (vma->vm_flags & VM_DROPPABLE))) {
> > dec_mm_counter(mm, MM_ANONPAGES);
> > goto discard;
> > }
> > @@ -1851,7 +1852,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> > * discarded. Remap the page to page table.
> > */
> > set_pte_at(mm, address, pvmw.pte, pteval);
> > - folio_set_swapbacked(folio);
> > + if (!(vma->vm_flags & VM_DROPPABLE))
> > + folio_set_swapbacked(folio);
> > ret = false;
> > page_vma_mapped_walk_done(&pvmw);
> > break;
> > --
> > 2.44.0
> >
> >
>
> This seems like an obvious question, but I can't seem to find a
> message asking this in the long history of this patchset: VM_DROPPABLE
> seems very close to MADV_FREE lazyfree memory.
Very different semantics and use case. For example, with MADV_FREE, if
you redirty the page by writing to it, the flag is cleared.
Jason
^ permalink raw reply
* Re: [PATCH v16 1/5] mm: add VM_DROPPABLE for designating always lazily freeable mappings
From: Frank van der Linden @ 2024-05-28 20:41 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, patches, tglx, linux-crypto, linux-api, x86,
Greg Kroah-Hartman, Adhemerval Zanella Netto, Carlos O'Donell,
Florian Weimer, Arnd Bergmann, Jann Horn, Christian Brauner,
David Hildenbrand, linux-mm
In-Reply-To: <20240528122352.2485958-2-Jason@zx2c4.com>
On Tue, May 28, 2024 at 5:24 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> The vDSO getrandom() implementation works with a buffer allocated with a
> new system call that has certain requirements:
>
> - It shouldn't be written to core dumps.
> * Easy: VM_DONTDUMP.
> - It should be zeroed on fork.
> * Easy: VM_WIPEONFORK.
>
> - It shouldn't be written to swap.
> * Uh-oh: mlock is rlimited.
> * Uh-oh: mlock isn't inherited by forks.
>
> - It shouldn't reserve actual memory, but it also shouldn't crash when
> page faulting in memory if none is available
> * Uh-oh: MAP_NORESERVE respects vm.overcommit_memory=2.
> * Uh-oh: VM_NORESERVE means segfaults.
>
> It turns out that the vDSO getrandom() function has three really nice
> characteristics that we can exploit to solve this problem:
>
> 1) Due to being wiped during fork(), the vDSO code is already robust to
> having the contents of the pages it reads zeroed out midway through
> the function's execution.
>
> 2) In the absolute worst case of whatever contingency we're coding for,
> we have the option to fallback to the getrandom() syscall, and
> everything is fine.
>
> 3) The buffers the function uses are only ever useful for a maximum of
> 60 seconds -- a sort of cache, rather than a long term allocation.
>
> These characteristics mean that we can introduce VM_DROPPABLE, which
> has the following semantics:
>
> a) It never is written out to swap.
> b) Under memory pressure, mm can just drop the pages (so that they're
> zero when read back again).
> c) If there's not enough memory to service a page fault, it's not fatal.
> d) It is inherited by fork.
> e) It doesn't count against the mlock budget, since nothing is locked.
>
> This is fairly simple to implement, with the one snag that we have to
> use 64-bit VM_* flags, but this shouldn't be a problem, since the only
> consumers will probably be 64-bit anyway.
>
> This way, allocations used by vDSO getrandom() can use:
>
> VM_DROPPABLE | VM_DONTDUMP | VM_WIPEONFORK | VM_NORESERVE
>
> And there will be no problem with OOMing, crashing on overcommitment,
> using memory when not in use, not wiping on fork(), coredumps, or
> writing out to swap.
>
> Cc: linux-mm@kvack.org
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> fs/proc/task_mmu.c | 3 +++
> include/linux/mm.h | 8 ++++++++
> include/trace/events/mmflags.h | 7 +++++++
> mm/Kconfig | 3 +++
> mm/memory.c | 4 ++++
> mm/mempolicy.c | 3 +++
> mm/mprotect.c | 2 +-
> mm/rmap.c | 8 +++++---
> 8 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index e5a5f015ff03..b5a59e57bde1 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -706,6 +706,9 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma)
> #endif /* CONFIG_HAVE_ARCH_USERFAULTFD_MINOR */
> #ifdef CONFIG_X86_USER_SHADOW_STACK
> [ilog2(VM_SHADOW_STACK)] = "ss",
> +#endif
> +#ifdef CONFIG_NEED_VM_DROPPABLE
> + [ilog2(VM_DROPPABLE)] = "dp",
> #endif
> };
> size_t i;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 9849dfda44d4..5978cb4cc21c 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -321,12 +321,14 @@ extern unsigned int kobjsize(const void *objp);
> #define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit architectures */
> #define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit architectures */
> #define VM_HIGH_ARCH_BIT_5 37 /* bit only usable on 64-bit architectures */
> +#define VM_HIGH_ARCH_BIT_6 38 /* bit only usable on 64-bit architectures */
> #define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
> #define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
> #define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
> #define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
> #define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
> #define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
> +#define VM_HIGH_ARCH_6 BIT(VM_HIGH_ARCH_BIT_6)
> #endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
>
> #ifdef CONFIG_ARCH_HAS_PKEYS
> @@ -357,6 +359,12 @@ extern unsigned int kobjsize(const void *objp);
> # define VM_SHADOW_STACK VM_NONE
> #endif
>
> +#ifdef CONFIG_NEED_VM_DROPPABLE
> +# define VM_DROPPABLE VM_HIGH_ARCH_6
> +#else
> +# define VM_DROPPABLE VM_NONE
> +#endif
> +
> #if defined(CONFIG_X86)
> # define VM_PAT VM_ARCH_1 /* PAT reserves whole VMA at once (x86) */
> #elif defined(CONFIG_PPC)
> diff --git a/include/trace/events/mmflags.h b/include/trace/events/mmflags.h
> index e46d6e82765e..fab7848df50a 100644
> --- a/include/trace/events/mmflags.h
> +++ b/include/trace/events/mmflags.h
> @@ -165,6 +165,12 @@ IF_HAVE_PG_ARCH_X(arch_3)
> # define IF_HAVE_UFFD_MINOR(flag, name)
> #endif
>
> +#ifdef CONFIG_NEED_VM_DROPPABLE
> +# define IF_HAVE_VM_DROPPABLE(flag, name) {flag, name},
> +#else
> +# define IF_HAVE_VM_DROPPABLE(flag, name)
> +#endif
> +
> #define __def_vmaflag_names \
> {VM_READ, "read" }, \
> {VM_WRITE, "write" }, \
> @@ -197,6 +203,7 @@ IF_HAVE_VM_SOFTDIRTY(VM_SOFTDIRTY, "softdirty" ) \
> {VM_MIXEDMAP, "mixedmap" }, \
> {VM_HUGEPAGE, "hugepage" }, \
> {VM_NOHUGEPAGE, "nohugepage" }, \
> +IF_HAVE_VM_DROPPABLE(VM_DROPPABLE, "droppable" ) \
> {VM_MERGEABLE, "mergeable" } \
>
> #define show_vma_flags(flags) \
> diff --git a/mm/Kconfig b/mm/Kconfig
> index b4cb45255a54..6cd65ea4b3ad 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -1056,6 +1056,9 @@ config ARCH_USES_HIGH_VMA_FLAGS
> bool
> config ARCH_HAS_PKEYS
> bool
> +config NEED_VM_DROPPABLE
> + select ARCH_USES_HIGH_VMA_FLAGS
> + bool
>
> config ARCH_USES_PG_ARCH_X
> bool
> diff --git a/mm/memory.c b/mm/memory.c
> index b5453b86ec4b..57b03fc73159 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5689,6 +5689,10 @@ vm_fault_t handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>
> lru_gen_exit_fault();
>
> + /* If the mapping is droppable, then errors due to OOM aren't fatal. */
> + if (vma->vm_flags & VM_DROPPABLE)
> + ret &= ~VM_FAULT_OOM;
> +
> if (flags & FAULT_FLAG_USER) {
> mem_cgroup_exit_user_fault();
> /*
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index aec756ae5637..a66289f1d931 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -2300,6 +2300,9 @@ struct folio *vma_alloc_folio_noprof(gfp_t gfp, int order, struct vm_area_struct
> pgoff_t ilx;
> struct page *page;
>
> + if (vma->vm_flags & VM_DROPPABLE)
> + gfp |= __GFP_NOWARN | __GFP_NORETRY;
> +
> pol = get_vma_policy(vma, addr, order, &ilx);
> page = alloc_pages_mpol_noprof(gfp | __GFP_COMP, order,
> pol, ilx, numa_node_id());
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 94878c39ee32..88ff3ecc08a1 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -622,7 +622,7 @@ mprotect_fixup(struct vma_iterator *vmi, struct mmu_gather *tlb,
> may_expand_vm(mm, oldflags, nrpages))
> return -ENOMEM;
> if (!(oldflags & (VM_ACCOUNT|VM_WRITE|VM_HUGETLB|
> - VM_SHARED|VM_NORESERVE))) {
> + VM_SHARED|VM_NORESERVE|VM_DROPPABLE))) {
> charged = nrpages;
> if (security_vm_enough_memory_mm(mm, charged))
> return -ENOMEM;
> diff --git a/mm/rmap.c b/mm/rmap.c
> index e8fc5ecb59b2..d873a3f06506 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1397,7 +1397,8 @@ void folio_add_new_anon_rmap(struct folio *folio, struct vm_area_struct *vma,
> VM_WARN_ON_FOLIO(folio_test_hugetlb(folio), folio);
> VM_BUG_ON_VMA(address < vma->vm_start ||
> address + (nr << PAGE_SHIFT) > vma->vm_end, vma);
> - __folio_set_swapbacked(folio);
> + if (!(vma->vm_flags & VM_DROPPABLE))
> + __folio_set_swapbacked(folio);
> __folio_set_anon(folio, vma, address, true);
>
> if (likely(!folio_test_large(folio))) {
> @@ -1841,7 +1842,7 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> * plus the rmap(s) (dropped by discard:).
> */
> if (ref_count == 1 + map_count &&
> - !folio_test_dirty(folio)) {
> + (!folio_test_dirty(folio) || (vma->vm_flags & VM_DROPPABLE))) {
> dec_mm_counter(mm, MM_ANONPAGES);
> goto discard;
> }
> @@ -1851,7 +1852,8 @@ static bool try_to_unmap_one(struct folio *folio, struct vm_area_struct *vma,
> * discarded. Remap the page to page table.
> */
> set_pte_at(mm, address, pvmw.pte, pteval);
> - folio_set_swapbacked(folio);
> + if (!(vma->vm_flags & VM_DROPPABLE))
> + folio_set_swapbacked(folio);
> ret = false;
> page_vma_mapped_walk_done(&pvmw);
> break;
> --
> 2.44.0
>
>
This seems like an obvious question, but I can't seem to find a
message asking this in the long history of this patchset: VM_DROPPABLE
seems very close to MADV_FREE lazyfree memory.
Could those functionalities be folded in to one?
- Frank
^ permalink raw reply
* Re: [PATCH v16 0/5] implement getrandom() in vDSO
From: Jason A. Donenfeld @ 2024-05-28 14:46 UTC (permalink / raw)
To: linux-kernel, patches, tglx
Cc: linux-crypto, linux-api, x86, Greg Kroah-Hartman,
Adhemerval Zanella Netto, Carlos O'Donell, Florian Weimer,
Arnd Bergmann, Jann Horn, Christian Brauner, David Hildenbrand
In-Reply-To: <20240528122352.2485958-1-Jason@zx2c4.com>
I've rebased Adhemerval's glibc patches for this and put them here:
https://git.zx2c4.com/glibc/log/?h=vdso
If you're running systemd, you may want to whitelist the syscall in
order to make use of it:
https://github.com/seccomp/libseccomp/pull/395
https://github.com/systemd/systemd/pull/25519
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox