| Message ID | 20240507033814.57906-1-liweishi@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND] drm/virtio: fix memory leak of vbuf | expand |
Hi Weishi, kernel test robot noticed the following build warnings: [auto build test WARNING on drm-misc/drm-misc-next] [also build test WARNING on drm/drm-next linus/master v6.9-rc7 next-20240508] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Weishi-Li/drm-virtio-fix-memory-leak-of-vbuf/20240507-114452 base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next patch link: https://lore.kernel.org/r/20240507033814.57906-1-liweishi%40kylinos.cn patch subject: [PATCH] [PATCH RESEND] drm/virtio: fix memory leak of vbuf config: i386-buildonly-randconfig-001-20240508 (https://download.01.org/0day-ci/archive/20240509/202405090747.y8ofUE7r-lkp@intel.com/config) compiler: clang version 18.1.4 (https://github.com/llvm/llvm-project e6c3289804a67ea0bb6a86fadbe454dd93b8d855) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240509/202405090747.y8ofUE7r-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202405090747.y8ofUE7r-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/gpu/drm/virtio/virtgpu_vq.c:474:13: warning: variable 'notify' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized] 474 | } else if (ret < 0) { | ^~~~~~~ drivers/gpu/drm/virtio/virtgpu_vq.c:487:6: note: uninitialized use occurs here 487 | if (notify) | ^~~~~~ drivers/gpu/drm/virtio/virtgpu_vq.c:474:9: note: remove the 'if' if its condition is always false 474 | } else if (ret < 0) { | ^~~~~~~~~~~~~~ 475 | free_vbuf(vgdev, vbuf); | ~~~~~~~~~~~~~~~~~~~~~~~ 476 | } else { | ~~~~~~ drivers/gpu/drm/virtio/virtgpu_vq.c:455:13: note: initialize the variable 'notify' to silence this warning 455 | bool notify; | ^ | = 0 1 warning generated. vim +474 drivers/gpu/drm/virtio/virtgpu_vq.c 448 449 static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev, 450 struct virtio_gpu_vbuffer *vbuf) 451 { 452 struct virtqueue *vq = vgdev->cursorq.vq; 453 struct scatterlist *sgs[1], ccmd; 454 int idx, ret, outcnt; 455 bool notify; 456 457 if (!drm_dev_enter(vgdev->ddev, &idx)) { 458 free_vbuf(vgdev, vbuf); 459 return; 460 } 461 462 sg_init_one(&ccmd, vbuf->buf, vbuf->size); 463 sgs[0] = &ccmd; 464 outcnt = 1; 465 466 spin_lock(&vgdev->cursorq.qlock); 467 retry: 468 ret = virtqueue_add_sgs(vq, sgs, outcnt, 0, vbuf, GFP_ATOMIC); 469 if (ret == -ENOSPC) { 470 spin_unlock(&vgdev->cursorq.qlock); 471 wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt); 472 spin_lock(&vgdev->cursorq.qlock); 473 goto retry; > 474 } else if (ret < 0) { 475 free_vbuf(vgdev, vbuf); 476 } else { 477 vbuf->seqno = ++vgdev->cursorq.seqno; 478 trace_virtio_gpu_cmd_queue(vq, 479 virtio_gpu_vbuf_ctrl_hdr(vbuf), 480 vbuf->seqno); 481 482 notify = virtqueue_kick_prepare(vq); 483 } 484 485 spin_unlock(&vgdev->cursorq.qlock); 486 487 if (notify) 488 virtqueue_notify(vq); 489 490 drm_dev_exit(idx); 491 } 492
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index b1a00c0c25a7..e90751cc97f2 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -356,12 +356,14 @@ static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev, ret = virtqueue_add_sgs(vq, sgs, outcnt, incnt, vbuf, GFP_ATOMIC); WARN_ON(ret); + if (ret < 0 && ret != -ENOSPC) { + free_vbuf(vgdev, vbuf); + } else { + vbuf->seqno = ++vgdev->ctrlq.seqno; + trace_virtio_gpu_cmd_queue(vq, virtio_gpu_vbuf_ctrl_hdr(vbuf), vbuf->seqno); - vbuf->seqno = ++vgdev->ctrlq.seqno; - trace_virtio_gpu_cmd_queue(vq, virtio_gpu_vbuf_ctrl_hdr(vbuf), vbuf->seqno); - - atomic_inc(&vgdev->pending_commands); - + atomic_inc(&vgdev->pending_commands); + } spin_unlock(&vgdev->ctrlq.qlock); drm_dev_exit(idx); @@ -469,6 +471,8 @@ static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev, wait_event(vgdev->cursorq.ack_queue, vq->num_free >= outcnt); spin_lock(&vgdev->cursorq.qlock); goto retry; + } else if (ret < 0) { + free_vbuf(vgdev, vbuf); } else { vbuf->seqno = ++vgdev->cursorq.seqno; trace_virtio_gpu_cmd_queue(vq,
Both virtio_gpu_queue_ctrl_buffer and virtio_gpu_queue_cursor use virtqueue_add_sgs to upload the structure virtio_gpu_vbuffer * vbuf to virtqueue. However, when the vbuf fails to upload and virtqueue_add_sgs returns -EIO or -ENOMEM, the vbuf will not be able to be free by virtio_gpu_dequeue_*_func, resulting in a continuous increase in memory allocated to vgdev ->vbufs. Therefore, make virtio_gpu_queue_ctrl_sgs and virtio_gpu_queue_cursor free vbuf directly after virtqueue_add_sgs returns -EIO or -ENOMEM. Signed-off-by: Weishi Li <liweishi@kylinos.cn> --- drivers/gpu/drm/virtio/virtgpu_vq.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)