[PATCH] drm/ras: Fix memory leak in doit_reply_value() error path

[PATCH] drm/ras: Fix memory leak in doit_reply_value() error path

[Shuicheng Lin]

If get_node_error_counter() fails after genlmsg_iput() has succeeded,
the allocated msg sk_buff is leaked. Refactor the error paths to use
goto labels, fixing the leak and eliminating duplicate cleanup code.

Fixes: c36218dc49f5 ("drm/ras: Introduce the DRM RAS infrastructure over generic netlink")
Cc: Riana Tauro <riana.tauro@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Shuicheng Lin <shuicheng.lin@intel.com>
---
 drivers/gpu/drm/drm_ras.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_ras.c b/drivers/gpu/drm/drm_ras.c
index b2fa5ab86d87..e98a9e74cfd3 100644
--- a/drivers/gpu/drm/drm_ras.c
+++ b/drivers/gpu/drm/drm_ras.c
@@ -195,25 +195,28 @@ static int doit_reply_value(struct genl_info *info, u32 node_id,
 
 	hdr = genlmsg_iput(msg, info);
 	if (!hdr) {
-		nlmsg_free(msg);
-		return -EMSGSIZE;
+		ret = -EMSGSIZE;
+		goto err_free_msg;
 	}
 
 	ret = get_node_error_counter(node_id, error_id,
 				     &error_name, &value);
 	if (ret)
-		return ret;
+		goto err_cancel_msg;
 
 	ret = msg_reply_value(msg, error_id, error_name, value);
-	if (ret) {
-		genlmsg_cancel(msg, hdr);
-		nlmsg_free(msg);
-		return ret;
-	}
+	if (ret)
+		goto err_cancel_msg;
 
 	genlmsg_end(msg, hdr);
 
 	return genlmsg_reply(msg, info);
+
+err_cancel_msg:
+	genlmsg_cancel(msg, hdr);
+err_free_msg:
+	nlmsg_free(msg);
+	return ret;
 }
 
 /**
-- 
2.34.1

Claude review: drm/ras: Fix memory leak in doit_reply_value() error path

[Claude Code Review Bot]

Overall Series Review

Subject: drm/ras: Fix memory leak in doit_reply_value() error path
Author: Shuicheng Lin <shuicheng.lin@intel.com>
Patches: 1
Reviewed: 2026-03-09T08:03:42.495830

---

This is a single-patch fix for a memory leak in `doit_reply_value()` in `drivers/gpu/drm/drm_ras.c`. The fix is straightforward, correct, and well-structured. It converts the existing error handling from inline cleanup to a `goto`-based pattern, which is the standard kernel style for shared cleanup paths.

The core bug: when `get_node_error_counter()` fails after `genlmsg_iput()` has succeeded, the `msg` sk_buff is leaked because the old code did a bare `return ret` without freeing it. This is a legitimate bug.

**Verdict: Good patch, should be accepted.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/ras: Fix memory leak in doit_reply_value() error path

[Claude Code Review Bot]

Patch Review

**Bug identification is correct.** The original code at the `get_node_error_counter()` failure path:

```c
ret = get_node_error_counter(node_id, error_id,
                             &error_name, &value);
if (ret)
    return ret;
```

This leaks `msg` (allocated earlier via `genlmsg_new()`) and also fails to call `genlmsg_cancel()` to undo the `genlmsg_iput()`.

**The fix is correct and follows kernel conventions:**

1. The `err_cancel_msg` label properly calls `genlmsg_cancel(msg, hdr)` before falling through to `err_free_msg`, which calls `nlmsg_free(msg)`. This ordering is correct — cancel the genl message header first, then free the underlying skb.

2. The `err_free_msg` label (used when `genlmsg_iput()` fails) correctly only frees the msg without calling `genlmsg_cancel()`, since there's no valid `hdr` to cancel at that point.

3. The `msg_reply_value()` error path, which previously had its own inline `genlmsg_cancel()` + `nlmsg_free()`, now correctly shares the `err_cancel_msg` goto target, eliminating code duplication.

**Minor observation:** The `Fixes:` tag references `c36218dc49f5`, which is appropriate as it targets the commit that introduced the buggy code.

**No issues found.** The patch is clean, minimal, and correct.

---
Generated by Claude Code Patch Reviewer