dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
@ 2020-03-05  1:32 Gurchetan Singh
  2020-03-05  1:32 ` [PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..) Gurchetan Singh
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gurchetan Singh @ 2020-03-05  1:32 UTC (permalink / raw)
  To: dri-devel; +Cc: kraxel, Gurchetan Singh

A resource will be a shmem based resource or a (planned)
vram based resource, so it makes sense to factor out common fields
(resource handle, dumb).

v2: move mapped field to shmem object

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h    | 13 +++++++----
 drivers/gpu/drm/virtio/virtgpu_object.c | 31 ++++++++++++++-----------
 drivers/gpu/drm/virtio/virtgpu_vq.c     |  6 +++--
 3 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index ce73895cf74b..8e2027da6cce 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -69,16 +69,21 @@ struct virtio_gpu_object_params {
 struct virtio_gpu_object {
 	struct drm_gem_shmem_object base;
 	uint32_t hw_res_handle;
-
-	struct sg_table *pages;
-	uint32_t mapped;
-
 	bool dumb;
 	bool created;
 };
 #define gem_to_virtio_gpu_obj(gobj) \
 	container_of((gobj), struct virtio_gpu_object, base.base)
 
+struct virtio_gpu_object_shmem {
+	struct virtio_gpu_object base;
+	struct sg_table *pages;
+	uint32_t mapped;
+};
+
+#define to_virtio_gpu_shmem(virtio_gpu_object) \
+	container_of((virtio_gpu_object), struct virtio_gpu_object_shmem, base)
+
 struct virtio_gpu_object_array {
 	struct ww_acquire_ctx ticket;
 	struct list_head next;
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index c5cad949eb8d..1f8b062bb9eb 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -65,16 +65,17 @@ static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t
 void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 {
 	struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
+	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
-	if (bo->pages) {
-		if (bo->mapped) {
+	if (shmem->pages) {
+		if (shmem->mapped) {
 			dma_unmap_sg(vgdev->vdev->dev.parent,
-				     bo->pages->sgl, bo->mapped,
+				     shmem->pages->sgl, shmem->mapped,
 				     DMA_TO_DEVICE);
-			bo->mapped = 0;
+			shmem->mapped = 0;
 		}
-		sg_free_table(bo->pages);
-		bo->pages = NULL;
+		sg_free_table(shmem->pages);
+		shmem->pages = NULL;
 		drm_gem_shmem_unpin(&bo->base.base);
 	}
 	virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
@@ -133,6 +134,7 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
 					unsigned int *nents)
 {
 	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 	struct scatterlist *sg;
 	int si, ret;
 
@@ -140,19 +142,20 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
 	if (ret < 0)
 		return -EINVAL;
 
-	bo->pages = drm_gem_shmem_get_sg_table(&bo->base.base);
-	if (!bo->pages) {
+	shmem->pages = drm_gem_shmem_get_sg_table(&bo->base.base);
+	if (!shmem->pages) {
 		drm_gem_shmem_unpin(&bo->base.base);
 		return -EINVAL;
 	}
 
 	if (use_dma_api) {
-		bo->mapped = dma_map_sg(vgdev->vdev->dev.parent,
-					bo->pages->sgl, bo->pages->nents,
-					DMA_TO_DEVICE);
-		*nents = bo->mapped;
+		shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
+					   shmem->pages->sgl,
+					   shmem->pages->nents,
+					   DMA_TO_DEVICE);
+		*nents = shmem->mapped;
 	} else {
-		*nents = bo->pages->nents;
+		*nents = shmem->pages->nents;
 	}
 
 	*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
@@ -162,7 +165,7 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
 		return -ENOMEM;
 	}
 
-	for_each_sg(bo->pages->sgl, sg, *nents, si) {
+	for_each_sg(shmem->pages->sgl, sg, *nents, si) {
 		(*ents)[si].addr = cpu_to_le64(use_dma_api
 					       ? sg_dma_address(sg)
 					       : sg_phys(sg));
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5e2375e0f7bb..73854915ec34 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -600,10 +600,11 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
 	struct virtio_gpu_transfer_to_host_2d *cmd_p;
 	struct virtio_gpu_vbuffer *vbuf;
 	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
 	if (use_dma_api)
 		dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-				       bo->pages->sgl, bo->pages->nents,
+				       shmem->pages->sgl, shmem->pages->nents,
 				       DMA_TO_DEVICE);
 
 	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
@@ -1015,10 +1016,11 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
 	struct virtio_gpu_transfer_host_3d *cmd_p;
 	struct virtio_gpu_vbuffer *vbuf;
 	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
+	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
 	if (use_dma_api)
 		dma_sync_sg_for_device(vgdev->vdev->dev.parent,
-				       bo->pages->sgl, bo->pages->nents,
+				       shmem->pages->sgl, shmem->pages->nents,
 				       DMA_TO_DEVICE);
 
 	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
-- 
2.25.1.481.gfbce0eb801-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..)
  2020-03-05  1:32 [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gurchetan Singh
@ 2020-03-05  1:32 ` Gurchetan Singh
  2020-03-09  9:47 ` [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gerd Hoffmann
  2020-03-19  9:32 ` Jiri Slaby
  2 siblings, 0 replies; 6+ messages in thread
From: Gurchetan Singh @ 2020-03-05  1:32 UTC (permalink / raw)
  To: dri-devel; +Cc: kraxel, Gurchetan Singh

This function can be reused for hostmem objects.

v2: move virtio_gpu_is_shmem() check to virtio_gpu_cleanup_object()
v3: use-after free fix

Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
---
 drivers/gpu/drm/virtio/virtgpu_drv.h    |  2 +-
 drivers/gpu/drm/virtio/virtgpu_object.c | 33 ++++++++++++++-----------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 8e2027da6cce..c1824bdf2418 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -371,7 +371,7 @@ int virtio_gpu_object_create(struct virtio_gpu_device *vgdev,
 			     struct virtio_gpu_object **bo_ptr,
 			     struct virtio_gpu_fence *fence);
 
-bool virtio_gpu_is_shmem(struct drm_gem_object *obj);
+bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo);
 
 /* virtgpu_prime.c */
 struct drm_gem_object *virtgpu_gem_prime_import_sg_table(
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
index 1f8b062bb9eb..cc65e8b3ec8b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -65,21 +65,26 @@ static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t
 void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
 {
 	struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
-	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
 
-	if (shmem->pages) {
-		if (shmem->mapped) {
-			dma_unmap_sg(vgdev->vdev->dev.parent,
-				     shmem->pages->sgl, shmem->mapped,
-				     DMA_TO_DEVICE);
-			shmem->mapped = 0;
+	virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
+	if (virtio_gpu_is_shmem(bo)) {
+		struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
+
+		if (shmem->pages) {
+			if (shmem->mapped) {
+				dma_unmap_sg(vgdev->vdev->dev.parent,
+					     shmem->pages->sgl, shmem->mapped,
+					     DMA_TO_DEVICE);
+				shmem->mapped = 0;
+			}
+
+			sg_free_table(shmem->pages);
+			shmem->pages = NULL;
+			drm_gem_shmem_unpin(&bo->base.base);
 		}
-		sg_free_table(shmem->pages);
-		shmem->pages = NULL;
-		drm_gem_shmem_unpin(&bo->base.base);
+
+		drm_gem_shmem_free_object(&bo->base.base);
 	}
-	virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
-	drm_gem_shmem_free_object(&bo->base.base);
 }
 
 static void virtio_gpu_free_object(struct drm_gem_object *obj)
@@ -110,9 +115,9 @@ static const struct drm_gem_object_funcs virtio_gpu_shmem_funcs = {
 	.mmap = drm_gem_shmem_mmap,
 };
 
-bool virtio_gpu_is_shmem(struct drm_gem_object *obj)
+bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo)
 {
-	return obj->funcs == &virtio_gpu_shmem_funcs;
+	return bo->base.base.funcs == &virtio_gpu_shmem_funcs;
 }
 
 struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
-- 
2.25.1.481.gfbce0eb801-goog

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
  2020-03-05  1:32 [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gurchetan Singh
  2020-03-05  1:32 ` [PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..) Gurchetan Singh
@ 2020-03-09  9:47 ` Gerd Hoffmann
  2020-03-19  9:32 ` Jiri Slaby
  2 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-03-09  9:47 UTC (permalink / raw)
  To: Gurchetan Singh; +Cc: dri-devel

On Wed, Mar 04, 2020 at 05:32:11PM -0800, Gurchetan Singh wrote:
> A resource will be a shmem based resource or a (planned)
> vram based resource, so it makes sense to factor out common fields
> (resource handle, dumb).
> 
> v2: move mapped field to shmem object

Pushed to drm-misc-next.

thanks,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
  2020-03-05  1:32 [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gurchetan Singh
  2020-03-05  1:32 ` [PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..) Gurchetan Singh
  2020-03-09  9:47 ` [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gerd Hoffmann
@ 2020-03-19  9:32 ` Jiri Slaby
  2020-03-19  9:47   ` Gerd Hoffmann
  2 siblings, 1 reply; 6+ messages in thread
From: Jiri Slaby @ 2020-03-19  9:32 UTC (permalink / raw)
  To: Gurchetan Singh, dri-devel; +Cc: kraxel

On 05. 03. 20, 2:32, Gurchetan Singh wrote:
> A resource will be a shmem based resource or a (planned)
> vram based resource, so it makes sense to factor out common fields
> (resource handle, dumb).
> 
> v2: move mapped field to shmem object

Hi,

I bisected the slab-out-of-bounds below to this patch. Is it known?

> [drm] pci: virtio-vga detected at 0000:00:0a.0
> virtio-pci 0000:00:0a.0: vgaarb: deactivate vga console
> [drm] features: -virgl +edid
> [drm] number of scanouts: 1
> [drm] number of cap sets: 0
> [drm] Initialized virtio_gpu 0.1.0 0 for virtio6 on minor 0
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in virtio_gpu_object_create+0x938/0xa10
> Write of size 8 at addr ffff8880c321b5c8 by task swapper/0/1
>
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.6.0-rc2+ #38
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
> Call Trace:
> dump_stack (/dev/shm/jslaby/linux/lib/dump_stack.c:120) 
> print_address_description.constprop.0 (/dev/shm/jslaby/linux/mm/kasan/report.c:375) 
> __kasan_report.cold (/dev/shm/jslaby/linux/mm/kasan/report.c:507) 
> kasan_report (/dev/shm/jslaby/linux/./arch/x86/include/asm/smap.h:69 /dev/shm/jslaby/linux/mm/kasan/common.c:642) 
> virtio_gpu_object_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_object.c:145 /dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_object.c:224) 
> virtio_gpu_gem_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_gem.c:42) 
> virtio_gpu_mode_dumb_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_gem.c:84) 
> drm_client_framebuffer_create (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_client.c:268 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_client.c:412) 
> drm_fb_helper_generic_probe (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2041) 
> __drm_fb_helper_initial_config_and_unlock (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:1588 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:1746) 
> drm_fbdev_client_hotplug (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2134) 
> drm_fbdev_generic_setup (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2212 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2185) 
> virtio_gpu_probe (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_drv.c:128) 
> virtio_dev_probe (/dev/shm/jslaby/linux/drivers/virtio/virtio.c:248) 
> really_probe (/dev/shm/jslaby/linux/drivers/base/dd.c:551) 
> driver_probe_device (/dev/shm/jslaby/linux/drivers/base/dd.c:724) 
> device_driver_attach (/dev/shm/jslaby/linux/drivers/base/dd.c:998) 
> __driver_attach (/dev/shm/jslaby/linux/drivers/base/dd.c:1077) 
> bus_for_each_dev (/dev/shm/jslaby/linux/drivers/base/bus.c:305) 
> bus_add_driver (/dev/shm/jslaby/linux/drivers/base/bus.c:623) 
> driver_register (/dev/shm/jslaby/linux/drivers/base/driver.c:171) 
> do_one_initcall (/dev/shm/jslaby/linux/init/main.c:1146) 
> kernel_init_freeable (/dev/shm/jslaby/linux/init/main.c:1218 /dev/shm/jslaby/linux/init/main.c:1235 /dev/shm/jslaby/linux/init/main.c:1255 /dev/shm/jslaby/linux/init/main.c:1439) 
> kernel_init (/dev/shm/jslaby/linux/init/main.c:1348) 
> ret_from_fork (/dev/shm/jslaby/linux/arch/x86/entry/entry_64.S:358) 
> 
> Allocated by task 1:
> save_stack (/dev/shm/jslaby/linux/mm/kasan/common.c:72) 
> __kasan_kmalloc.constprop.0 (/dev/shm/jslaby/linux/mm/kasan/common.c:80 /dev/shm/jslaby/linux/mm/kasan/common.c:515 /dev/shm/jslaby/linux/mm/kasan/common.c:488) 
> virtio_gpu_create_object (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_object.c:124) 
> drm_gem_shmem_create (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_gem_shmem_helper.c:58) 
> virtio_gpu_object_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_object.c:193) 
> virtio_gpu_gem_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_gem.c:42) 
> virtio_gpu_mode_dumb_create (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_gem.c:84) 
> drm_client_framebuffer_create (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_client.c:268 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_client.c:412) 
> drm_fb_helper_generic_probe (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2041) 
> __drm_fb_helper_initial_config_and_unlock (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:1588 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:1746) 
> drm_fbdev_client_hotplug (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2134) 
> drm_fbdev_generic_setup (/dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2212 /dev/shm/jslaby/linux/drivers/gpu/drm/drm_fb_helper.c:2185) 
> virtio_gpu_probe (/dev/shm/jslaby/linux/drivers/gpu/drm/virtio/virtgpu_drv.c:128) 
> virtio_dev_probe (/dev/shm/jslaby/linux/drivers/virtio/virtio.c:248) 
> really_probe (/dev/shm/jslaby/linux/drivers/base/dd.c:551) 
> driver_probe_device (/dev/shm/jslaby/linux/drivers/base/dd.c:724) 
> device_driver_attach (/dev/shm/jslaby/linux/drivers/base/dd.c:998) 
> __driver_attach (/dev/shm/jslaby/linux/drivers/base/dd.c:1077) 
> bus_for_each_dev (/dev/shm/jslaby/linux/drivers/base/bus.c:305) 
> bus_add_driver (/dev/shm/jslaby/linux/drivers/base/bus.c:623) 
> driver_register (/dev/shm/jslaby/linux/drivers/base/driver.c:171) 
> do_one_initcall (/dev/shm/jslaby/linux/init/main.c:1146) 
> kernel_init_freeable (/dev/shm/jslaby/linux/init/main.c:1218 /dev/shm/jslaby/linux/init/main.c:1235 /dev/shm/jslaby/linux/init/main.c:1255 /dev/shm/jslaby/linux/init/main.c:1439) 
> kernel_init (/dev/shm/jslaby/linux/init/main.c:1348) 
> ret_from_fork (/dev/shm/jslaby/linux/arch/x86/entry/entry_64.S:358) 
> 
> Freed by task 1:
> save_stack (/dev/shm/jslaby/linux/mm/kasan/common.c:72) 
> __kasan_slab_free (/dev/shm/jslaby/linux/mm/kasan/common.c:478) 
> kfree (/dev/shm/jslaby/linux/mm/slub.c:1477 /dev/shm/jslaby/linux/mm/slub.c:3024 /dev/shm/jslaby/linux/mm/slub.c:3976) 
> consume_skb (/dev/shm/jslaby/linux/net/core/skbuff.c:681 /dev/shm/jslaby/linux/net/core/skbuff.c:839 /dev/shm/jslaby/linux/net/core/skbuff.c:833) 
> netlink_broadcast_filtered (/dev/shm/jslaby/linux/./include/asm-generic/atomic-instrumented.h:747 /dev/shm/jslaby/linux/net/netlink/af_netlink.c:465 /dev/shm/jslaby/linux/net/netlink/af_netlink.c:1514) 
> netlink_broadcast (/dev/shm/jslaby/linux/net/netlink/af_netlink.c:1534) 
> genl_ctrl_event (/dev/shm/jslaby/linux/net/netlink/genetlink.c:1042) 
> genl_register_family (/dev/shm/jslaby/linux/net/netlink/genetlink.c:380 /dev/shm/jslaby/linux/net/netlink/genetlink.c:322) 
> acpi_event_init (/dev/shm/jslaby/linux/drivers/acpi/event.c:178) 
> do_one_initcall (/dev/shm/jslaby/linux/init/main.c:1146) 
> kernel_init_freeable (/dev/shm/jslaby/linux/init/main.c:1218 /dev/shm/jslaby/linux/init/main.c:1235 /dev/shm/jslaby/linux/init/main.c:1255 /dev/shm/jslaby/linux/init/main.c:1439) 
> kernel_init (/dev/shm/jslaby/linux/init/main.c:1348) 
> ret_from_fork (/dev/shm/jslaby/linux/arch/x86/entry/entry_64.S:358) 













> Signed-off-by: Gurchetan Singh <gurchetansingh@chromium.org>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drv.h    | 13 +++++++----
>  drivers/gpu/drm/virtio/virtgpu_object.c | 31 ++++++++++++++-----------
>  drivers/gpu/drm/virtio/virtgpu_vq.c     |  6 +++--
>  3 files changed, 30 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index ce73895cf74b..8e2027da6cce 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -69,16 +69,21 @@ struct virtio_gpu_object_params {
>  struct virtio_gpu_object {
>  	struct drm_gem_shmem_object base;
>  	uint32_t hw_res_handle;
> -
> -	struct sg_table *pages;
> -	uint32_t mapped;
> -
>  	bool dumb;
>  	bool created;
>  };
>  #define gem_to_virtio_gpu_obj(gobj) \
>  	container_of((gobj), struct virtio_gpu_object, base.base)
>  
> +struct virtio_gpu_object_shmem {
> +	struct virtio_gpu_object base;
> +	struct sg_table *pages;
> +	uint32_t mapped;
> +};
> +
> +#define to_virtio_gpu_shmem(virtio_gpu_object) \
> +	container_of((virtio_gpu_object), struct virtio_gpu_object_shmem, base)
> +
>  struct virtio_gpu_object_array {
>  	struct ww_acquire_ctx ticket;
>  	struct list_head next;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c b/drivers/gpu/drm/virtio/virtgpu_object.c
> index c5cad949eb8d..1f8b062bb9eb 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -65,16 +65,17 @@ static void virtio_gpu_resource_id_put(struct virtio_gpu_device *vgdev, uint32_t
>  void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
>  {
>  	struct virtio_gpu_device *vgdev = bo->base.base.dev->dev_private;
> +	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
>  
> -	if (bo->pages) {
> -		if (bo->mapped) {
> +	if (shmem->pages) {
> +		if (shmem->mapped) {
>  			dma_unmap_sg(vgdev->vdev->dev.parent,
> -				     bo->pages->sgl, bo->mapped,
> +				     shmem->pages->sgl, shmem->mapped,
>  				     DMA_TO_DEVICE);
> -			bo->mapped = 0;
> +			shmem->mapped = 0;
>  		}
> -		sg_free_table(bo->pages);
> -		bo->pages = NULL;
> +		sg_free_table(shmem->pages);
> +		shmem->pages = NULL;
>  		drm_gem_shmem_unpin(&bo->base.base);
>  	}
>  	virtio_gpu_resource_id_put(vgdev, bo->hw_res_handle);
> @@ -133,6 +134,7 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
>  					unsigned int *nents)
>  {
>  	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
> +	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
>  	struct scatterlist *sg;
>  	int si, ret;
>  
> @@ -140,19 +142,20 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
>  	if (ret < 0)
>  		return -EINVAL;
>  
> -	bo->pages = drm_gem_shmem_get_sg_table(&bo->base.base);
> -	if (!bo->pages) {
> +	shmem->pages = drm_gem_shmem_get_sg_table(&bo->base.base);
> +	if (!shmem->pages) {
>  		drm_gem_shmem_unpin(&bo->base.base);
>  		return -EINVAL;
>  	}
>  
>  	if (use_dma_api) {
> -		bo->mapped = dma_map_sg(vgdev->vdev->dev.parent,
> -					bo->pages->sgl, bo->pages->nents,
> -					DMA_TO_DEVICE);
> -		*nents = bo->mapped;
> +		shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
> +					   shmem->pages->sgl,
> +					   shmem->pages->nents,
> +					   DMA_TO_DEVICE);
> +		*nents = shmem->mapped;
>  	} else {
> -		*nents = bo->pages->nents;
> +		*nents = shmem->pages->nents;
>  	}
>  
>  	*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
> @@ -162,7 +165,7 @@ static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,
>  		return -ENOMEM;
>  	}
>  
> -	for_each_sg(bo->pages->sgl, sg, *nents, si) {
> +	for_each_sg(shmem->pages->sgl, sg, *nents, si) {
>  		(*ents)[si].addr = cpu_to_le64(use_dma_api
>  					       ? sg_dma_address(sg)
>  					       : sg_phys(sg));
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 5e2375e0f7bb..73854915ec34 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -600,10 +600,11 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct virtio_gpu_device *vgdev,
>  	struct virtio_gpu_transfer_to_host_2d *cmd_p;
>  	struct virtio_gpu_vbuffer *vbuf;
>  	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
> +	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
>  
>  	if (use_dma_api)
>  		dma_sync_sg_for_device(vgdev->vdev->dev.parent,
> -				       bo->pages->sgl, bo->pages->nents,
> +				       shmem->pages->sgl, shmem->pages->nents,
>  				       DMA_TO_DEVICE);
>  
>  	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> @@ -1015,10 +1016,11 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct virtio_gpu_device *vgdev,
>  	struct virtio_gpu_transfer_host_3d *cmd_p;
>  	struct virtio_gpu_vbuffer *vbuf;
>  	bool use_dma_api = !virtio_has_iommu_quirk(vgdev->vdev);
> +	struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
>  
>  	if (use_dma_api)
>  		dma_sync_sg_for_device(vgdev->vdev->dev.parent,
> -				       bo->pages->sgl, bo->pages->nents,
> +				       shmem->pages->sgl, shmem->pages->nents,
>  				       DMA_TO_DEVICE);
>  
>  	cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
> 

thanks,
-- 
js
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
  2020-03-19  9:32 ` Jiri Slaby
@ 2020-03-19  9:47   ` Gerd Hoffmann
  2020-03-19  9:55     ` Jiri Slaby
  0 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2020-03-19  9:47 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: dri-devel, Gurchetan Singh

On Thu, Mar 19, 2020 at 10:32:25AM +0100, Jiri Slaby wrote:
> On 05. 03. 20, 2:32, Gurchetan Singh wrote:
> > A resource will be a shmem based resource or a (planned)
> > vram based resource, so it makes sense to factor out common fields
> > (resource handle, dumb).
> > 
> > v2: move mapped field to shmem object
> 
> Hi,
> 
> I bisected the slab-out-of-bounds below to this patch. Is it known?

No.  I suspect sizeof(virtio_gpu_object) instead of
sizeof(virtio_gpu_object_shmem) for allocation, I'll have a look ...

cheers,
  Gerd

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
  2020-03-19  9:47   ` Gerd Hoffmann
@ 2020-03-19  9:55     ` Jiri Slaby
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Slaby @ 2020-03-19  9:55 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: dri-devel, Gurchetan Singh

On 19. 03. 20, 10:47, Gerd Hoffmann wrote:
> On Thu, Mar 19, 2020 at 10:32:25AM +0100, Jiri Slaby wrote:
>> On 05. 03. 20, 2:32, Gurchetan Singh wrote:
>>> A resource will be a shmem based resource or a (planned)
>>> vram based resource, so it makes sense to factor out common fields
>>> (resource handle, dumb).
>>>
>>> v2: move mapped field to shmem object
>>
>> Hi,
>>
>> I bisected the slab-out-of-bounds below to this patch. Is it known?
> 
> No.  I suspect sizeof(virtio_gpu_object) instead of
> sizeof(virtio_gpu_object_shmem) for allocation, I'll have a look ...

Ah, this?
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -123,15 +123,17 @@ bool virtio_gpu_is_shmem(struct virtio_gpu_object *bo)
 struct drm_gem_object *virtio_gpu_create_object(struct drm_device *dev,
                                                size_t size)
 {
-       struct virtio_gpu_object *bo;
+       struct virtio_gpu_object_shmem *shmem;
+       struct drm_gem_shmem_object *dshmem;

-       bo = kzalloc(sizeof(*bo), GFP_KERNEL);
-       if (!bo)
+       shmem = kzalloc(sizeof(*shmem), GFP_KERNEL);
+       if (!shmem)
                return NULL;

-       bo->base.base.funcs = &virtio_gpu_shmem_funcs;
-       bo->base.map_cached = true;
-       return &bo->base.base;
+       dshmem = &shmem->base.base;
+       dshmem->base.funcs = &virtio_gpu_shmem_funcs;
+       dshmem->map_cached = true;
+       return &dshmem->base;
 }

 static int virtio_gpu_object_shmem_init(struct virtio_gpu_device *vgdev,

thanks,
-- 
js
suse labs
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2020-03-19  9:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-05  1:32 [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gurchetan Singh
2020-03-05  1:32 ` [PATCH v3 2/2] drm/virtio: add case for shmem objects in virtio_gpu_cleanup_object(..) Gurchetan Singh
2020-03-09  9:47 ` [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object Gerd Hoffmann
2020-03-19  9:32 ` Jiri Slaby
2020-03-19  9:47   ` Gerd Hoffmann
2020-03-19  9:55     ` Jiri Slaby

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).