linux-kernel - [PATCH RFC] drm/radeon: fix the warning for radeon_cs

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>] [day] [month] [year] [list]

Message-ID: <20250705155914.806693-1-abinashsinghlalotra@gmail.com>
Date: Sat,  5 Jul 2025 21:29:14 +0530
From: Abinash Singh <abinashlalotra@...il.com>
To: alexander.deucher@....com,
	christian.koenig@....com,
	airlied@...il.com,
	simona@...ll.ch
Cc: amd-gfx@...ts.freedesktop.org,
	dri-devel@...ts.freedesktop.org,
	linux-kernel@...r.kernel.org,
	Abinash Singh <abinashsinghlalotra@...il.com>
Subject: [PATCH RFC] drm/radeon: fix the warning for radeon_cs_ioctl()

Fix the following compiler warning about excessive stack usage:

 drivers/gpu/drm/radeon/radeon_cs.c:669:5: warning: stack frame size (1200) exceeds limit (1024) in 'radeon_cs_ioctl' [-Wframe-larger-than]

The warning is caused by a large objects like  `struct radeon_cs_parser`
 being allocated on the stack. This patch changes the parser to be 
 heap-allocated via `kzalloc()`, thus reducing stack pressure. 
 It uses the __free() attribute  to ensure automatic 
 cleanup on return.

This change should not affect functionality, as all usage of the
 `parser` remains intact and is now accessed via a pointer.


Signed-off-by: Abinash Singh <abinashsinghlalotra@...il.com>
---
the radeon_cs_ioctl() consumes around 1200 bytes on stack.
It was confirmed using -fstack-usage
....................................
divers/gpu/drm/radeon/radeon_cs.c:669:radeon_cs_ioctl  1200    static
drivers/gpu/drm/radeon/radeon_cs.c:416:radeon_cs_parser_fini    56      static
...............................................

After applying this patch the stack size effectivly reduces to 656 bytes
...............................................
drivers/gpu/drm/radeon/radeon_cs.c:669:radeon_cs_ioctl  656     static
drivers/gpu/drm/radeon/radeon_cs.c:416:radeon_cs_parser_fini    56      static
.....................................................

This will eliminate the build warning .But will this dynamic allocation with kzalloc()
affect performance as compared to stack allocation + memset . Or will this way we will
suffer greater penalty ?
 drivers/gpu/drm/radeon/radeon_cs.c | 34 ++++++++++++++++--------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index b8e6202f1d5b..ca7fad842db7 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -669,9 +669,12 @@ static int radeon_cs_ib_fill(struct radeon_device *rdev, struct radeon_cs_parser
 int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 {
 	struct radeon_device *rdev = dev->dev_private;
-	struct radeon_cs_parser parser;
+	struct radeon_cs_parser *parser __free(kfree) = NULL;
 	int r;
-
+
+	parser = kzalloc(sizeof(*parser), GFP_KERNEL);
+	if (!parser)
+		return -ENOMEM;
 	down_read(&rdev->exclusive_lock);
 	if (!rdev->accel_working) {
 		up_read(&rdev->exclusive_lock);
@@ -685,46 +688,45 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		return r;
 	}
 	/* initialize parser */
-	memset(&parser, 0, sizeof(struct radeon_cs_parser));
-	parser.filp = filp;
-	parser.rdev = rdev;
-	parser.dev = rdev->dev;
-	parser.family = rdev->family;
-	r = radeon_cs_parser_init(&parser, data);
+	parser->filp = filp;
+	parser->rdev = rdev;
+	parser->dev = rdev->dev;
+	parser->family = rdev->family;
+	r = radeon_cs_parser_init(parser, data);
 	if (r) {
 		DRM_ERROR("Failed to initialize parser !\n");
-		radeon_cs_parser_fini(&parser, r);
+		radeon_cs_parser_fini(parser, r);
 		up_read(&rdev->exclusive_lock);
 		r = radeon_cs_handle_lockup(rdev, r);
 		return r;
 	}

-	r = radeon_cs_ib_fill(rdev, &parser);
+	r = radeon_cs_ib_fill(rdev, parser);
 	if (!r) {
-		r = radeon_cs_parser_relocs(&parser);
+		r = radeon_cs_parser_relocs(parser);
 		if (r && r != -ERESTARTSYS)
 			DRM_ERROR("Failed to parse relocation %d!\n", r);
 	}

 	if (r) {
-		radeon_cs_parser_fini(&parser, r);
+		radeon_cs_parser_fini(parser, r);
 		up_read(&rdev->exclusive_lock);
 		r = radeon_cs_handle_lockup(rdev, r);
 		return r;
 	}

-	trace_radeon_cs(&parser);
+	trace_radeon_cs(parser);

-	r = radeon_cs_ib_chunk(rdev, &parser);
+	r = radeon_cs_ib_chunk(rdev, parser);
 	if (r) {
 		goto out;
 	}
-	r = radeon_cs_ib_vm_chunk(rdev, &parser);
+	r = radeon_cs_ib_vm_chunk(rdev, parser);
 	if (r) {
 		goto out;
 	}
 out:
-	radeon_cs_parser_fini(&parser, r);
+	radeon_cs_parser_fini(parser, r);
 	up_read(&rdev->exclusive_lock);
 	r = radeon_cs_handle_lockup(rdev, r);
 	return r;
--q
2.43.0