Re: [PATCH 09/17] mm: export access_remote_vm() symbol

From: Lorenzo Stoakes <lstoakes@gmail.com>
To: Alistair Popple <apopple@nvidia.com>
Cc: "Yu, Fenghua" <fenghua.yu@intel.com>,
	Vinod Koul <vkoul@kernel.org>,
	"Jiang, Dave" <dave.jiang@intel.com>,
	"dmaengine@vger.kernel.org" <dmaengine@vger.kernel.org>,
	"Zhu, Tony" <tony.zhu@intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	Christoph Hellwig <hch@infradead.org>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>
Subject: Re: [PATCH 09/17] mm: export access_remote_vm() symbol
Date: Wed, 4 Jan 2023 21:05:11 +0000	[thread overview]
Message-ID: <Y7XqBwewsUnAP4uP@lucifer> (raw)
In-Reply-To: <Y7XZ8zY3KIRDlu/f@lucifer>

On Wed, Jan 04, 2023 at 07:56:35PM +0000, Lorenzo Stoakes wrote:
> Another question is - why can't we:-
>
> 1. mmgrab() [or safely assume we already have a reference] + mmget_not_zero()
> 2. acquire mm read lock to stop VMAs disappearing underneath us and pin pages with get_user_pages_remote()
> 3. copy what we need using e.g. copy_from_user()/copy_to_user()
> 4. unwind

OK looking at __access_remote_vm() I just accidentally described exactly what it
does other than step 1 :)

Perhaps then the answer is a wrapper that gets the reference before
invoking __access_remote_vm()? I guess we could assume grab there.

It strikes me that access_remote_vm() being quite literally a pass through to
__access_remote_vm() means we could:-

a. change all callers of access_remote_vm() to use __access_remote_vm()
b. Update access_remote_vm() to be safer
c. finally, export access_remote_vm()

e.g.:-

int access_remote_vm(struct mm_struct *mm, unsigned long addr,
		void *buf, int len, unsigned int gup_flags)
{
	int ret;

	if (!mmget_not_zero(mm))
		return 0;

	ret = __access_remote_vm(mm, addr, buf, len, gup_flags);

	mmput(mm);
}
EXPORT_SYMBOL_GPL(access_remote_vm)