On Thu, May 16, 2019 at 4:20 PM Oleksandr Natalenko
<oleksandr@xxxxxxxxxx> wrote:
> On Thu, May 16, 2019 at 12:00:24PM +0200, Jann Horn wrote:
> > On Thu, May 16, 2019 at 11:43 AM Oleksandr Natalenko
> > <oleksandr@xxxxxxxxxx> wrote:
> > > Use previously introduced remote madvise knob to mark task's
> > > anonymous memory as mergeable.
> > >
> > > To force merging task's VMAs, "merge" hint is used:
> > >
> > > # echo merge > /proc/<pid>/madvise
> > >
> > > Force unmerging is done similarly:
> > >
> > > # echo unmerge > /proc/<pid>/madvise
> > >
> > > To achieve this, previously introduced ksm_madvise_*() helpers
> > > are used.
> >
> > Why does this not require PTRACE_MODE_ATTACH_FSCREDS to the target
> > process? Enabling KSM on another process is hazardous because it
> > significantly increases the attack surface for side channels.
> >
> > (Note that if you change this to require PTRACE_MODE_ATTACH_FSCREDS,
> > you'll want to use mm_access() in the ->open handler and drop the mm
> > in ->release. mm_access() from a ->write handler is not permitted.)
>
> Sounds reasonable. So, something similar to what mem_open() & friends do
> now:
>
> static int madvise_open(...)
> ...
> struct task_struct *task = get_proc_task(inode);
> ...
> if (task) {
> mm = mm_access(task, PTRACE_MODE_ATTACH_FSCREDS);
> put_task_struct(task);
> if (!IS_ERR_OR_NULL(mm)) {
> mmgrab(mm);
> mmput(mm);
> ...
>
> Then:
>
> static ssize_t madvise_write(...)
> ...
> if (!mmget_not_zero(mm))
> goto out;
>
> down_write(&mm->mmap_sem);
> if (!mmget_still_valid(mm))
> goto skip_mm;
> ...
> skip_mm:
> up_write(&mm->mmap_sem);
>
> mmput(mm);
> out:
> return ...;
>
> And, finally:
>
> static int madvise_release(...)
> ...
> mmdrop(mm);
> ...
>
> Right?
Yeah, that looks reasonable.
