linux-kernel - [PATCH] drm/panthor: Fix memory leak in panthor_ioctl_group

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241113-panthor-fix-gcq-bailout-v1-1-654307254d68@google.com>
Date: Wed, 13 Nov 2024 22:03:39 +0100
From: Jann Horn <jannh@...gle.com>
To: Boris Brezillon <boris.brezillon@...labora.com>, 
 Steven Price <steven.price@....com>, Liviu Dudau <liviu.dudau@....com>
Cc: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>, 
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
 Mary Guillemard <mary.guillemard@...labora.com>, 
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
 stable@...r.kernel.org, Jann Horn <jannh@...gle.com>
Subject: [PATCH] drm/panthor: Fix memory leak in
 panthor_ioctl_group_create()

When bailing out due to group_priority_permit() failure, the queue_args
need to be freed. Fix it by rearranging the function to use the
goto-on-error pattern, such that the success case flows straight without
indentation while error cases jump forward to cleanup.

Cc: stable@...r.kernel.org
Fixes: 5f7762042f8a ("drm/panthor: Restrict high priorities on group_create")
Signed-off-by: Jann Horn <jannh@...gle.com>
---
testcase:
```
#include <err.h>
#include <fcntl.h>
#include <stddef.h>
#include <sys/ioctl.h>
#include <drm/panthor_drm.h>

#define SYSCHK(x) ({          \
  typeof(x) __res = (x);      \
  if (__res == (typeof(x))-1) \
    err(1, "SYSCHK(" #x ")"); \
  __res;                      \
})

#define GPU_PATH "/dev/dri/by-path/platform-fb000000.gpu-card"

int main(void) {
  int fd = SYSCHK(open(GPU_PATH, O_RDWR));

  while (1) {
    struct drm_panthor_queue_create qc[16] = {};
    struct drm_panthor_group_create gc = {
      .queues = {
        .stride = sizeof(struct drm_panthor_queue_create),
        .count = 16,
        .array = (unsigned long)qc
      },
      .priority = PANTHOR_GROUP_PRIORITY_HIGH+1/*invalid*/
    };
    ioctl(fd, DRM_IOCTL_PANTHOR_GROUP_CREATE, &gc);
  }
}
```

I have tested that without this patch, after running the testcase for a
few seconds and then manually killing it, 2G of RAM in kmalloc-128 have
been leaked. With the patch applied, the memory leak is gone.

(By the way, get_maintainer.pl suggests that I also send this patch to
the general DRM maintainers and the DRM-misc maintainers; looking at
MAINTAINERS, it looks like it is normal that the general DRM maintainers
are listed for everything under drivers/gpu/, but DRM-misc has exclusion
rules for a bunch of drivers but not panthor. I don't know if that is
intentional.)
---
 drivers/gpu/drm/panthor/panthor_drv.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index c520f156e2d73f7e735f8bf2d6d8e8efacec9362..815c23cff25f305d884e8e3e263fa22888f7d5ce 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1032,14 +1032,15 @@ static int panthor_ioctl_group_create(struct drm_device *ddev, void *data,
 
 	ret = group_priority_permit(file, args->priority);
 	if (ret)
-		return ret;
+		goto out;
 
 	ret = panthor_group_create(pfile, args, queue_args);
-	if (ret >= 0) {
-		args->group_handle = ret;
-		ret = 0;
-	}
+	if (ret < 0)
+		goto out;
+	args->group_handle = ret;
+	ret = 0;
 
+out:
 	kvfree(queue_args);
 	return ret;
 }

---
base-commit: 9f8e716d46c68112484a23d1742d9ec725e082fc
change-id: 20241113-panthor-fix-gcq-bailout-2d9ac36590ed

-- 
Jann Horn <jannh@...gle.com>