So I like this series.
However, logically, I think this part in replace_mm_exe_file() no
longer makes sense:
On Mon, Aug 16, 2021 at 12:50 PM David Hildenbrand <david@xxxxxxxxxx> wrote:
>
> + /* Forbid mm->exe_file change if old file still mapped. */
> + old_exe_file = get_mm_exe_file(mm);
> + if (old_exe_file) {
> + mmap_read_lock(mm);
> + for (vma = mm->mmap; vma && !ret; vma = vma->vm_next) {
> + if (!vma->vm_file)
> + continue;
> + if (path_equal(&vma->vm_file->f_path,
> + &old_exe_file->f_path))
> + ret = -EBUSY;
> + }
> + mmap_read_unlock(mm);
> + fput(old_exe_file);
> + if (ret)
> + return ret;
> + }
and should just be removed.
NOTE! I think it makes sense within the context of this patch (where
you just move code around), but that it should then be removed in the
next patch that does that "always deny write access to current MM
exe_file" thing.
I just quoted it in the context of this patch, since the next patch
doesn't actually show this code any more.
In the *old* model - where the ETXTBUSY was about the mmap() of the
file - the above tests make sense.
But in the new model, walking the mappings just doesn't seem to be a
sensible operation any more. The mappings simply aren't what ETXTBUSY
is about in the new world order, and so doing that mapping walk seems
nonsensical.
Hmm?
Linus
