# Re: \[RESEND RFC PATCH 4/5\] mm/remote\_mapping: use a pidfd to access memory belonging to unrelated process

- From: Oleg Nesterov \<oleg@redhat.com\>
- Date: 2020-09-04 18:20:19 
- List: linux-api
- Page: https://ratatoskr.run/linux-api/2020/09/6503102
- Thread: https://ratatoskr.run/linux-mm/2020/09/6503085/t.md

```text
I didn't read this series. This is not my area and to be honest even
the API doesn't fit my head. I leave this to other reviewers.

Just a couple of nits after very quick glance.

On 09/04, Adalbert Lazăr wrote:
>
> +static vm_fault_t mirror_vm_fault(struct vm_fault *vmf)
> +{
...

> +	up_read(&current->mm->mmap_sem);

...

> +	down_read(&current->mm->mmap_sem);
> +
> +	/* expedite retry */
> +	if (mmu_interval_check_retry(&view->mmin, seq)) {
> +		put_page(req_page);
> +
> +		srcu_read_unlock(&fctx->fault_srcu, idx);
> +
> +		goto fault_retry;
> +	}
> +
> +	/* make sure the VMA hasn't gone away */
> +	vma = find_vma(current->mm, vmf->address);
> +	if (vma == vmf->vma) {

vmf->vma can go away, its memory can be freed and re-allocated as another
vma returned by find_vma() above.

> +int task_remote_map(struct task_struct *task, int fds[])
> +{

...

> +	fds[1] = anon_inode_getfd("[pidfd_mem.map]", &pidfd_mem_map_fops, fctx,
> +				  O_RDWR | O_CLOEXEC | O_LARGEFILE);
> +	if (fds[1] < 0) {
> +		ret = fds[1];
> +		goto out;
> +	}
> +	remote_file_context_get(fctx);
> +
> +	map = fget(fds[1]);

Another thread can close this file right after fd_install(). fget() can return
NULL or another unrelated file.

Oleg.

```