search for: nouveau_gem_ioctl_pushbuf

Date: Wed, 5 Apr 2023 18:38:54 +0200 The label ?out_prevalid? was used to jump to another pointer check despite of the detail in the implementation of the function ?nouveau_gem_ioctl_pushbuf? that it was determined already in one case that the corresponding variable contained an error pointer because of a failed call of the function ?u_memcpya?. Thus use an additional label. This issue was detected by using the Coccinelle software. Fixes: 2be65641642ef423f82162c3a5f28c754d1637d2 (&q...

...ned i; - reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc)); - if (IS_ERR(reloc)) - return PTR_ERR(reloc); - for (i = 0; i < req->nr_relocs; i++) { struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; struct drm_nouveau_gem_pushbuf_bo *b; @@ -691,11 +680,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, struct nouveau_drm *drm = nouveau_drm(dev); struct drm_nouveau_gem_pushbuf *req = data; struct drm_nouveau_gem_pushbuf_push *push; + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; struct drm_nouveau_gem_pushbuf_bo *bo; struct nouveau_channel *chan...

...t; changed IB size calculation to be less wasteful, but didn't take into account already existing off-by-one bugs :). So: - ib_max is the last entry, so we need to +1 when calculating number of free entries - nv50_dma_wait already does +1 (for FIRE_RING), so we don't need another +1 on nouveau_gem_ioctl_pushbuf side - there are 512 allocated IB entries (and it needs to be round number), so we can accept at most 511 entries from userspace (we need one for FIRE_RING) - fortunately userspace already flushes at 511, so nr_push check change won't have any impact Signed-off-by: Marcin Slusarz <mar...

...ned i; - reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc)); - if (IS_ERR(reloc)) - return PTR_ERR(reloc); - for (i = 0; i < req->nr_relocs; i++) { struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; struct drm_nouveau_gem_pushbuf_bo *b; @@ -693,11 +682,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, struct nouveau_drm *drm = nouveau_drm(dev); struct drm_nouveau_gem_pushbuf *req = data; struct drm_nouveau_gem_pushbuf_push *push; + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; struct drm_nouveau_gem_pushbuf_bo *bo; struct nouveau_channel *chan...

...ned i; - reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc)); - if (IS_ERR(reloc)) - return PTR_ERR(reloc); - for (i = 0; i < req->nr_relocs; i++) { struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; struct drm_nouveau_gem_pushbuf_bo *b; @@ -693,11 +682,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, struct nouveau_drm *drm = nouveau_drm(dev); struct drm_nouveau_gem_pushbuf *req = data; struct drm_nouveau_gem_pushbuf_push *push; + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; struct drm_nouveau_gem_pushbuf_bo *bo; struct nouveau_channel *chan...

...- DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_INIT, nouveau_svmm_init, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_AUTH|DRM_RENDER_ALLOW), - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nouveau_gem_ioctl_info, DRM_AUTH|DRM...

...0 +413,6 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan, INIT_LIST_HEAD(&op->gart_list); INIT_LIST_HEAD(&op->both_list); - ret = nouveau_fence_new(chan, &op->fence, false); - if (ret) - return ret; - if (nr_buffers == 0) return 0; @@ -541,6 +530,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, struct drm_nouveau_gem_pushbuf_bo *bo = NULL; struct nouveau_channel *chan; struct validate_op op; + struct nouveau_fence* fence = 0; uint32_t *pushbuf = NULL; int ret = 0, do_reloc = 0, i; @@ -597,7 +587,8 @@ nouveau_gem_ioctl_pushbuf(struct drm_devi...

...ned i; - reloc = u_memcpya(req->relocs, req->nr_relocs, sizeof(*reloc)); - if (IS_ERR(reloc)) - return PTR_ERR(reloc); - for (i = 0; i < req->nr_relocs; i++) { struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; struct drm_nouveau_gem_pushbuf_bo *b; @@ -691,11 +680,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, struct nouveau_drm *drm = nouveau_drm(dev); struct drm_nouveau_gem_pushbuf *req = data; struct drm_nouveau_gem_pushbuf_push *push; + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; struct drm_nouveau_gem_pushbuf_bo *bo; struct nouveau_channel *chan...

..., or enforces the assumption, and it's not true in practice. 0x7fffff is not the maximum possible push buffer size submitted for IB-mode DMA transfers, nor is it the maximum legal DMA transfer size (it is three bytes too large according to our reverse-engineering docs). 2) nouveau_gem_ioctl_pushbuf() still implicitly treats bit 23 of the pushbuffer buffer[].length field as NOUVEAU_GEM_PUSHBUF_NO_PREFETCH, despite previous commits moving the flag out of the length field for lower levels as this is an nv50-hardware-specific arrangement. It also conflicts with point 1 above as p...

Reduces object size ~4kb Joe Perches (2): drm/nouveau: Add new logging function nv_cli_printk drm/nouveau: Convert NV_PRINTK macros and uses to new nv_cli_<level> macros drivers/gpu/drm/nouveau/nouveau_abi16.c | 2 +- drivers/gpu/drm/nouveau/nouveau_chan.c | 12 +++---- drivers/gpu/drm/nouveau/nouveau_drm.c | 21 +++++++++++ drivers/gpu/drm/nouveau/nouveau_drv.h | 44

...t;relocs, req->nr_relocs, sizeof(*reloc)); > - if (IS_ERR(reloc)) > - return PTR_ERR(reloc); > - > for (i = 0; i < req->nr_relocs; i++) { > struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; > struct drm_nouveau_gem_pushbuf_bo *b; > @@ -691,11 +680,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, > struct nouveau_drm *drm = nouveau_drm(dev); > struct drm_nouveau_gem_pushbuf *req = data; > struct drm_nouveau_gem_pushbuf_push *push; > + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; > struct drm_nouveau_gem_pushbuf_bo *bo; >...

...t;relocs, req->nr_relocs, sizeof(*reloc)); > - if (IS_ERR(reloc)) > - return PTR_ERR(reloc); > - > for (i = 0; i < req->nr_relocs; i++) { > struct drm_nouveau_gem_pushbuf_reloc *r = &reloc[i]; > struct drm_nouveau_gem_pushbuf_bo *b; > @@ -693,11 +682,13 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, > struct nouveau_drm *drm = nouveau_drm(dev); > struct drm_nouveau_gem_pushbuf *req = data; > struct drm_nouveau_gem_pushbuf_push *push; > + struct drm_nouveau_gem_pushbuf_reloc *reloc = NULL; > struct drm_nouveau_gem_pushbuf_bo *bo; >...

...x108 [<ffffffa1f81af824>] refcount_dec_and_test+0x14/0x20 [<ffffffa1f847405c>] reservation_object_add_excl_fence+0x144/0x1e0 [<ffffff900113cce0>] nouveau_bo_fence+0x50/0x60 [nouveau] [<ffffff900113d1dc>] validate_fini_no_ticket+0xc4/0x190 [nouveau] [<ffffff900113e1fc>] nouveau_gem_ioctl_pushbuf+0x49c/0x1c78 [nouveau] [<ffffff9000ed84b8>] drm_ioctl+0x280/0x590 [drm] [<ffffff900113200c>] nouveau_drm_ioctl+0x8c/0x100 [nouveau] [<ffffffa1f7f2c910>] do_vfs_ioctl+0x130/0x9a0 [<ffffffa1f7f2d20c>] SyS_ioctl+0x8c/0xa0 [<ffffffa1f7cf3730>] el0_svc_naked+0x24/0x28 &quot...

From: Thierry Reding <treding at nvidia.com> This small series of patches implements support for waiting on and emitting fence FDs on kickoff. This enables explicit fencing and can be used for example to synchronize buffer accesses between the display engine and the GPU on Tegra. The first patch lays the groundwork by splitting up nouveau_fence_sync() to allow reuse. Patch 2 is where the

...INIT, nouveau_svmm_init, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_SVM_BIND, nouveau_svmm_bind, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_NEW, nouveau_gem_ioctl_new, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_PUSHBUF, nouveau_gem_ioctl_pushbuf, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_PREP, nouveau_gem_ioctl_cpu_prep, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_CPU_FINI, nouveau_gem_ioctl_cpu_fini, DRM_AUTH|DRM_RENDER_ALLOW), > - DRM_IOCTL_DEF_DRV(NOUVEAU_GEM_INFO, nou...

...#0: ffff8d3e562d30c0 (&cli->mutex){+.+.}-{4:4}, at: nouveau_abi16_get+0x2c/0x60 [nouveau] [ 1143.133756] #1: ffffa9a6c0c57d30 (reservation_ww_class_acquire){+.+.}-{0:0}, at: drm_ioctl_kernel+0x91/0xe0 [drm] [ 1143.133785] #2: ffff8d3e3dcef1a0 (reservation_ww_class_mutex){+.+.}-{4:4}, at: nouveau_gem_ioctl_pushbuf+0x63b/0x1cb0 [nouveau] [ 1143.133834] #3: ffff8d3e9ec9ea10 (krc.lock){-.-.}-{2:2}, at: kvfree_call_rcu+0x65/0x210 [ 1143.133845] stack backtrace: [ 1143.133850] CPU: 2 PID: 2015 Comm: X Kdump: loaded Tainted: G S E 5.9.0.g34d4ddd-preempt #2 [ 1143.133856] Hardware name: MEDION MS-7848...

If you're especially unlucky BOs would move around and their kmaps would end up pointing to something else in GART, then ioctl_pushbuf() would use the kmaps again corrupting textures or other pushbufs (the most noticeable symptom was a PFIFO_DMA_PUSHER from time to time). Signed-off-by: Francisco Jerez <currojerez at riseup.net> --- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 ++ 1

...x108 [<ffffffa1f81af824>] refcount_dec_and_test+0x14/0x20 [<ffffffa1f847405c>] reservation_object_add_excl_fence+0x144/0x1e0 [<ffffff900113cce0>] nouveau_bo_fence+0x50/0x60 [nouveau] [<ffffff900113d1dc>] validate_fini_no_ticket+0xc4/0x190 [nouveau] [<ffffff900113e1fc>] nouveau_gem_ioctl_pushbuf+0x49c/0x1c78 [nouveau] [<ffffff9000ed84b8>] drm_ioctl+0x280/0x590 [drm] [<ffffff900113200c>] nouveau_drm_ioctl+0x8c/0x100 [nouveau] [<ffffffa1f7f2c910>] do_vfs_ioctl+0x130/0x9a0 [<ffffffa1f7f2d20c>] SyS_ioctl+0x8c/0xa0 [<ffffffa1f7cf3730>] el0_svc_naked+0x24/0x28 &quot...

Found by sparse. Signed-off-by: Luca Barbieri <luca at luca-barbieri.com> --- drivers/gpu/drm/nouveau/nouveau_gem.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c index 03d8935..d7ace31 100644 --- a/drivers/gpu/drm/nouveau/nouveau_gem.c +++ b/drivers/gpu/drm/nouveau/nouveau_gem.c @@

..._USER(mem, userptr, size)) { + u_free(mem); return ERR_PTR(-EFAULT); } @@ -681,7 +697,7 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli, nouveau_bo_wr32(nvbo, r->reloc_bo_offset >> 2, data); } - kfree(reloc); + u_free(reloc); return ret; } @@ -743,7 +759,7 @@ nouveau_gem_ioctl_pushbuf(struct drm_device *dev, void *data, bo = u_memcpya(req->buffers, req->nr_buffers, sizeof(*bo)); if (IS_ERR(bo)) { - kfree(push); + u_free(push); return nouveau_abi16_put(abi16, PTR_ERR(bo)); } @@ -854,8 +870,8 @@ out: nouveau_fence_unref(&fence); out_prevalid: - kfree(...