On Wed, Feb 26, 2025 at 05:26:04PM +0100, Oleg Nesterov wrote: > On 02/24, jeffxu@xxxxxxxxxxxx wrote: > > > > Unlike other system mappings, the uprobe mapping is not > > established during program startup. However, its lifetime is the same > > as the process's lifetime. It could be sealed from creation. > > Agreed, VM_SEALED should be always for the "[uprobes]" vma, regardless > of config options. If you think this ought to be the case generally, then perhaps we should drop this patch from the commit and just do this separately as a permanent-on thing, if you are sure this is fine + want it? An aside - we _definitely_ cannot allow this -system mapping stuff- to be enabled without a config option, this is emphatic, for the reason that it breaks userspace and is only known-good on some arches. A config flag that checks arch gives a big warning saying 'hey this breaks userspace' means users use it knowing this to be the case. > > > ACK, > > but can't we do > > #ifdef CONFIG_64BIT > /* VM is sealed, in vm_flags */ > #define VM_SEALED _BITUL(63) > + #else > + #define VM_SEALED 0 > #endif This has been raised a few times. Jeff objects to this (for reasons I don't agree with, honestly) but it's been implemented in this way from the start (in order to catch the case of 32-bit systems trying to use mseal but it not happening). Anyway I agree, but I'm not going to push this at least in this series. > > and then simply > > vma = _install_special_mapping(mm, area->vaddr, PAGE_SIZE, > - VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO, > + VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO|VM_SEALED, > > ? Nah you'd have to do: > vma = _install_special_mapping(mm, area->vaddr, PAGE_SIZE, VM_EXEC|VM_MAYEXEC|VM_DONTCOPY|VM_IO #ifdef CONFIG_64BIT VM_SEALED #endif , Or something similar :) Beautiful no? > > But I am fine either way, feel free to ignore. > > Oleg. > Ideally We should hear what the security use case is here. I mean I'm less bothered with it behind a flag, but it seems odd that an attacker would break out of a sandbox into a process currently being debugged... seems unlikely. I'm not sure under what other circumstances this would be a problem. Jeff, Kees? Anyway as I said before, I don't overly object to this as-is, as long as you are ok with it Oleg and can absolutely confirm this will never break anything, you don't need to remap, unmap (until process teardown), adjust the VMA in any way etc.? A quick glance at uprobe code suggests it's fine.
