All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] mm/frame-vec: Drop gup_flags from get_vaddr_frames()
@ 2020-10-02 17:53 ` Daniel Vetter
  0 siblings, 0 replies; 156+ messages in thread
From: Daniel Vetter @ 2020-10-02 17:53 UTC (permalink / raw)
  To: DRI Development
  Cc: LKML, Daniel Vetter, Daniel Vetter, Inki Dae, Joonyoung Shim,
	Seung-Woo Kim, Kyungmin Park, Kukjin Kim, Krzysztof Kozlowski,
	Pawel Osciak, Marek Szyprowski, Tomasz Figa, Andrew Morton,
	Oded Gabbay, Omer Shpigelman, Tomer Tayar, Greg Kroah-Hartman,
	Pawel Piskorski, linux-arm-kernel, linux-samsung-soc,
	linux-media, linux-mm

FOLL_WRITE | FOLL_FORCE is really the only reasonable thing to do for
simple dma device that can't guarantee write protection. Which is also
what all the callers are using.

So just simplify this.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Pawel Osciak <pawel@osciak.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Tomasz Figa <tfiga@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oded Gabbay <oded.gabbay@gmail.com>
Cc: Omer Shpigelman <oshpigelman@habana.ai>
Cc: Tomer Tayar <ttayar@habana.ai>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Pawel Piskorski <ppiskorski@habana.ai>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-mm@kvack.org
---
 drivers/gpu/drm/exynos/exynos_drm_g2d.c           | 3 +--
 drivers/media/common/videobuf2/videobuf2-memops.c | 3 +--
 drivers/misc/habanalabs/common/memory.c           | 3 +--
 include/linux/mm.h                                | 2 +-
 mm/frame_vector.c                                 | 4 ++--
 5 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_g2d.c b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
index 967a5cdc120e..ac452842bab3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_g2d.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_g2d.c
@@ -480,8 +480,7 @@ static dma_addr_t *g2d_userptr_get_dma_addr(struct g2d_data *g2d,
 		goto err_free;
 	}
 
-	ret = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
-		g2d_userptr->vec);
+	ret = get_vaddr_frames(start, npages, g2d_userptr->vec);
 	if (ret != npages) {
 		DRM_DEV_ERROR(g2d->dev,
 			      "failed to get user pages from userptr.\n");
diff --git a/drivers/media/common/videobuf2/videobuf2-memops.c b/drivers/media/common/videobuf2/videobuf2-memops.c
index 6e9e05153f4e..9dd6c27162f4 100644
--- a/drivers/media/common/videobuf2/videobuf2-memops.c
+++ b/drivers/media/common/videobuf2/videobuf2-memops.c
@@ -40,7 +40,6 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
 	unsigned long first, last;
 	unsigned long nr;
 	struct frame_vector *vec;
-	unsigned int flags = FOLL_FORCE | FOLL_WRITE;
 
 	first = start >> PAGE_SHIFT;
 	last = (start + length - 1) >> PAGE_SHIFT;
@@ -48,7 +47,7 @@ struct frame_vector *vb2_create_framevec(unsigned long start,
 	vec = frame_vector_create(nr);
 	if (!vec)
 		return ERR_PTR(-ENOMEM);
-	ret = get_vaddr_frames(start & PAGE_MASK, nr, flags, vec);
+	ret = get_vaddr_frames(start & PAGE_MASK, nr, vec);
 	if (ret < 0)
 		goto out_destroy;
 	/* We accept only complete set of PFNs */
diff --git a/drivers/misc/habanalabs/common/memory.c b/drivers/misc/habanalabs/common/memory.c
index 5ff4688683fd..43b10aee8150 100644
--- a/drivers/misc/habanalabs/common/memory.c
+++ b/drivers/misc/habanalabs/common/memory.c
@@ -1287,8 +1287,7 @@ static int get_user_memory(struct hl_device *hdev, u64 addr, u64 size,
 		return -ENOMEM;
 	}
 
-	rc = get_vaddr_frames(start, npages, FOLL_FORCE | FOLL_WRITE,
-				userptr->vec);
+	rc = get_vaddr_frames(start, npages, userptr->vec);
 
 	if (rc != npages) {
 		dev_err(hdev->dev,
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 16b799a0522c..7d14aa2780d2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1757,7 +1757,7 @@ struct frame_vector {
 struct frame_vector *frame_vector_create(unsigned int nr_frames);
 void frame_vector_destroy(struct frame_vector *vec);
 int get_vaddr_frames(unsigned long start, unsigned int nr_pfns,
-		     unsigned int gup_flags, struct frame_vector *vec);
+		     struct frame_vector *vec);
 void put_vaddr_frames(struct frame_vector *vec);
 int frame_vector_to_pages(struct frame_vector *vec);
 void frame_vector_to_pfns(struct frame_vector *vec);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 10f82d5643b6..5d34c9047e9c 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -12,7 +12,6 @@
  * get_vaddr_frames() - map virtual addresses to pfns
  * @start:	starting user address
  * @nr_frames:	number of pages / pfns from start to map
- * @gup_flags:	flags modifying lookup behaviour
  * @vec:	structure which receives pages / pfns of the addresses mapped.
  *		It should have space for at least nr_frames entries.
  *
@@ -32,10 +31,11 @@
  * This function takes care of grabbing mmap_lock as necessary.
  */
 int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
-		     unsigned int gup_flags, struct frame_vector *vec)
+		     struct frame_vector *vec)
 {
 	struct mm_struct *mm = current->mm;
 	struct vm_area_struct *vma;
+	unsigned int gup_flags = FOLL_WRITE | FOLL_FORCE;
 	int ret = 0;
 	int err;
 	int locked;
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 156+ messages in thread
* Re: [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM
@ 2020-10-05 17:38 ` Jason Gunthorpe
  0 siblings, 0 replies; 156+ messages in thread
From: Jason Gunthorpe @ 2020-10-05 17:38 UTC (permalink / raw)
  To: andrew Morton, Daniel Vetter, linux-mm
  Cc: Hans Verkuil, Jan Kara, Mauro Carvalho Chehab, Mel Gorman,
	stable, Vlastimil Babka, John Hubbard, DRI Development, LKML,
	Dan Williams, Linux ARM, linux-samsung-soc, linux-media

When get_vaddr_frames() does its hacky follow_pfn() loop it should never
be allowed to extract a struct page from a normal VMA. This could allow a
serious use-after-free problem on any kernel memory.

Restrict this to only work on VMA's with one of VM_IO | VM_PFNMAP
set. This limits the use-after-free problem to only IO memory, which while
still serious, is an improvement.

Cc: stable@vger.kernel.org
Fixes: 8025e5ddf9c1 ("[media] mm: Provide new get_vaddr_frames() helper")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 mm/frame_vector.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index 10f82d5643b6de..26cb20544b6c37 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -99,6 +99,10 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames,
 		if (ret >= nr_frames || start < vma->vm_end)
 			break;
 		vma = find_vma_intersection(mm, start, start + 1);
+		if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
+			ret = -EINVAL;
+			goto out;
+		}
 	} while (vma && vma->vm_flags & (VM_IO | VM_PFNMAP));
 out:
 	if (locked)
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 156+ messages in thread

end of thread, other threads:[~2020-10-07 15:40 UTC | newest]

Thread overview: 156+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-02 17:53 [PATCH 1/2] mm/frame-vec: Drop gup_flags from get_vaddr_frames() Daniel Vetter
2020-10-02 17:53 ` Daniel Vetter
2020-10-02 17:53 ` Daniel Vetter
2020-10-02 17:53 ` [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM Daniel Vetter
2020-10-02 17:53   ` Daniel Vetter
2020-10-02 17:53   ` Daniel Vetter
2020-10-02 18:06   ` Jason Gunthorpe
2020-10-02 18:06     ` Jason Gunthorpe
2020-10-02 18:06     ` Jason Gunthorpe
2020-10-02 18:16     ` Daniel Vetter
2020-10-02 18:16       ` Daniel Vetter
2020-10-02 18:16       ` Daniel Vetter
2020-10-02 23:31       ` Jason Gunthorpe
2020-10-02 23:31         ` Jason Gunthorpe
2020-10-02 23:31         ` Jason Gunthorpe
2020-10-03  8:34         ` Oded Gabbay
2020-10-03  8:34           ` Oded Gabbay
2020-10-03  8:34           ` Oded Gabbay
2020-10-03  9:40         ` Daniel Vetter
2020-10-03  9:40           ` Daniel Vetter
2020-10-03  9:40           ` Daniel Vetter
2020-10-04 12:50           ` Jason Gunthorpe
2020-10-04 12:50             ` Jason Gunthorpe
2020-10-04 12:50             ` Jason Gunthorpe
2020-10-04 16:09             ` Daniel Vetter
2020-10-04 16:09               ` Daniel Vetter
2020-10-04 16:09               ` Daniel Vetter
2020-10-05 17:28               ` Jason Gunthorpe
2020-10-05 17:28                 ` Jason Gunthorpe
2020-10-05 17:28                 ` Jason Gunthorpe
2020-10-05 18:16                 ` Daniel Vetter
2020-10-05 18:16                   ` Daniel Vetter
2020-10-05 18:16                   ` Daniel Vetter
2020-10-05 18:37                   ` Jason Gunthorpe
2020-10-05 18:37                     ` Jason Gunthorpe
2020-10-05 18:37                     ` Jason Gunthorpe
2020-10-05 18:54                     ` Daniel Vetter
2020-10-05 18:54                       ` Daniel Vetter
2020-10-05 18:54                       ` Daniel Vetter
2020-10-05 22:43                       ` Daniel Vetter
2020-10-05 22:43                         ` Daniel Vetter
2020-10-05 22:43                         ` Daniel Vetter
2020-10-05 23:41                         ` Jason Gunthorpe
2020-10-05 23:41                           ` Jason Gunthorpe
2020-10-05 23:41                           ` Jason Gunthorpe
2020-10-06  6:23                           ` Daniel Vetter
2020-10-06  6:23                             ` Daniel Vetter
2020-10-06  6:23                             ` Daniel Vetter
2020-10-06 12:26                             ` Jason Gunthorpe
2020-10-06 12:26                               ` Jason Gunthorpe
2020-10-06 12:26                               ` Jason Gunthorpe
2020-10-06 13:08                               ` Daniel Vetter
2020-10-06 13:08                                 ` Daniel Vetter
2020-10-06 13:08                                 ` Daniel Vetter
2020-10-07 10:47           ` Marek Szyprowski
2020-10-07 10:47             ` Marek Szyprowski
2020-10-07 10:47             ` Marek Szyprowski
2020-10-07 12:01             ` Daniel Vetter
2020-10-07 12:01               ` Daniel Vetter
2020-10-07 12:01               ` Daniel Vetter
2020-10-07 12:33               ` Marek Szyprowski
2020-10-07 12:33                 ` Marek Szyprowski
2020-10-07 12:33                 ` Marek Szyprowski
2020-10-07 12:44                 ` Jason Gunthorpe
2020-10-07 12:44                   ` Jason Gunthorpe
2020-10-07 12:44                   ` Jason Gunthorpe
2020-10-07 12:47                   ` Tomasz Figa
2020-10-07 12:47                     ` Tomasz Figa
2020-10-07 12:47                     ` Tomasz Figa
2020-10-07 12:58                     ` Daniel Vetter
2020-10-07 12:58                       ` Daniel Vetter
2020-10-07 12:58                       ` Daniel Vetter
2020-10-07 13:06                       ` Jason Gunthorpe
2020-10-07 13:06                         ` Jason Gunthorpe
2020-10-07 13:06                         ` Jason Gunthorpe
2020-10-07 13:34                         ` Tomasz Figa
2020-10-07 13:34                           ` Tomasz Figa
2020-10-07 13:34                           ` Tomasz Figa
2020-10-07 13:42                           ` Jason Gunthorpe
2020-10-07 13:42                             ` Jason Gunthorpe
2020-10-07 13:42                             ` Jason Gunthorpe
2020-10-07 14:08                           ` Daniel Vetter
2020-10-07 14:08                             ` Daniel Vetter
2020-10-07 14:08                             ` Daniel Vetter
2020-10-07 14:11                             ` Tomasz Figa
2020-10-07 14:11                               ` Tomasz Figa
2020-10-07 14:11                               ` Tomasz Figa
2020-10-07 14:22                               ` Daniel Vetter
2020-10-07 14:22                                 ` Daniel Vetter
2020-10-07 14:22                                 ` Daniel Vetter
2020-10-07 15:05                                 ` Tomasz Figa
2020-10-07 15:05                                   ` Tomasz Figa
2020-10-07 15:05                                   ` Tomasz Figa
2020-10-07 14:58                               ` Jason Gunthorpe
2020-10-07 14:58                                 ` Jason Gunthorpe
2020-10-07 14:58                                 ` Jason Gunthorpe
2020-10-07 13:06         ` Tomasz Figa
2020-10-07 13:06           ` Tomasz Figa
2020-10-07 13:06           ` Tomasz Figa
2020-10-07 13:14           ` Jason Gunthorpe
2020-10-07 13:14             ` Jason Gunthorpe
2020-10-07 13:14             ` Jason Gunthorpe
2020-10-05 15:03     ` Jan Kara
2020-10-05 15:03       ` Jan Kara
2020-10-05 15:03       ` Jan Kara
2020-10-02 22:39   ` John Hubbard
2020-10-02 22:39     ` John Hubbard
2020-10-02 22:39     ` John Hubbard
2020-10-03  9:45     ` Daniel Vetter
2020-10-03  9:45       ` Daniel Vetter
2020-10-03  9:45       ` Daniel Vetter
2020-10-03 22:52       ` John Hubbard
2020-10-03 22:52         ` John Hubbard
2020-10-03 22:52         ` John Hubbard
2020-10-03 23:24         ` Jason Gunthorpe
2020-10-03 23:24           ` Jason Gunthorpe
2020-10-03 23:24           ` Jason Gunthorpe
2020-10-04 11:20           ` Daniel Vetter
2020-10-04 11:20             ` Daniel Vetter
2020-10-04 11:20             ` Daniel Vetter
2020-10-05 17:35             ` Jason Gunthorpe
2020-10-05 17:35               ` Jason Gunthorpe
2020-10-05 17:35               ` Jason Gunthorpe
2020-10-02 18:22 ` [PATCH 1/2] mm/frame-vec: Drop gup_flags from get_vaddr_frames() Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 18:22   ` Tomasz Figa
2020-10-02 19:21   ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-02 19:21     ` Oded Gabbay
2020-10-05 17:38 [PATCH 2/2] mm/frame-vec: use FOLL_LONGTERM Jason Gunthorpe
2020-10-05 17:38 ` Jason Gunthorpe
2020-10-05 17:47 ` Jason Gunthorpe
2020-10-05 17:47   ` Jason Gunthorpe
2020-10-05 17:47   ` Jason Gunthorpe
2020-10-06  3:36   ` Andrew Morton
2020-10-06  3:36     ` Andrew Morton
2020-10-06  3:36     ` Andrew Morton
2020-10-06 11:57     ` Jason Gunthorpe
2020-10-06 11:57       ` Jason Gunthorpe
2020-10-06 11:57       ` Jason Gunthorpe
2020-10-05 17:53 ` Jan Kara
2020-10-05 17:53   ` Jan Kara
2020-10-05 17:53   ` Jan Kara
2020-10-05 17:57   ` Jason Gunthorpe
2020-10-05 17:57     ` Jason Gunthorpe
2020-10-05 17:57     ` Jason Gunthorpe
2020-10-05 18:16     ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-05 18:16       ` Daniel Vetter
2020-10-06 11:56     ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter
2020-10-06 11:56       ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.