Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
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
Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
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
Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
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 :00:0a.0 > virtio-pci :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 8880c321b5c8 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_driv
Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
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
[PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
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 --- 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,