On 02/26, Lorenzo Stoakes wrote: > > 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? See below... > An aside - we _definitely_ cannot allow this -system mapping stuff- to be > enabled without a config option, This is clear. But as for uprobes in particular I do think that VM_SEALED is always fine. Do we really want it? I dunno. If a task unmaps its "[uprobes]" vma it will crash when it hits the uprobes bp next time. Unless the probed insn can be emulated and it is not ret-probe. Do we really care? Again, I don't know. Should this change come as a separate patch? I don't understand why it should but I am fine either way. In short. please do what you think is right, VM_SEALED can't hurt uprobes ;) > > #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 OK, > > 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 > , Why??? With the proposed change above VM_SEALED == 0 if !CONFIG_64BIT. Oleg.
