Am Montag, dem 01.03.2021 um 10:52 +0100 schrieb Daniel Vetter:
Nothing checks userptr.ro except this call to pup_fast, which means there's nothing actually preventing userspace from writing to this. Which means you can just read-only mmap any file you want, userptr it and then write to it with the gpu. Not good.
I agree about the "not good" part.
The right way to handle this is FOLL_WRITE | FOLL_FORCE, which will break any COW mappings and update tracking for MAY_WRITE mappings so there's no exploit and the vm isn't confused about what's going on. For any legit use case there's no difference from what userspace can observe and do.
This however seems pretty heavy handed. Does this mean we do a full COW cycle of the userpages on BO creation? This most likely kills a lot of the performance benefits that one might seek by using userptr. If that's the case I might still take this patch for stable, but then we should rather just disallow writable GPU mappings to this BO.
Regards, Lucas
Cc: stable@vger.kernel.org Cc: John Hubbard jhubbard@nvidia.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com Cc: Lucas Stach l.stach@pengutronix.de Cc: Russell King linux+etnaviv@armlinux.org.uk Cc: Christian Gmeiner christian.gmeiner@gmail.com Cc: etnaviv@lists.freedesktop.org
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index 6d38c5c17f23..a9e696d05b33 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -689,7 +689,7 @@ static int etnaviv_gem_userptr_get_pages(struct etnaviv_gem_object *etnaviv_obj) struct page **pages = pvec + pinned;
ret = pin_user_pages_fast(ptr, num_pages,
!userptr->ro ? FOLL_WRITE : 0, pages);
FOLL_WRITE | FOLL_FORCE, pages);
if (ret < 0) { unpin_user_pages(pvec, pinned); kvfree(pvec);