All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Rob Clark <robdclark@chromium.org>,
	Rob Clark <robin.clark@oss.qualcomm.com>,
	Sasha Levin <sashal@kernel.org>,
	lumag@kernel.org, linux-arm-msm@vger.kernel.org,
	dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org
Subject: [PATCH AUTOSEL 6.1 02/15] drm/msm: Fix another leak in the submit error path
Date: Tue, 24 Jun 2025 00:12:25 -0400	[thread overview]
Message-ID: <20250624041238.84580-2-sashal@kernel.org> (raw)
In-Reply-To: <20250624041238.84580-1-sashal@kernel.org>

From: Rob Clark <robdclark@chromium.org>

[ Upstream commit f681c2aa8676a890eacc84044717ab0fd26e058f ]

put_unused_fd() doesn't free the installed file, if we've already done
fd_install().  So we need to also free the sync_file.

Signed-off-by: Rob Clark <robdclark@chromium.org>
Patchwork: https://patchwork.freedesktop.org/patch/653583/
Signed-off-by: Rob Clark <robin.clark@oss.qualcomm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my analysis, **YES**, this commit should be backported to
stable kernel trees.

## Extensive Explanation:

This commit fixes a **resource leak** in the MSM DRM driver's submit
error path, which is a clear bug that affects users and meets the stable
tree criteria.

### The Bug:

Looking at the code changes in `msm_gem_submit.c`:

1. **Before the fix**: When an error occurs after `fd_install()` has
   been called (line 865 in the context), the error path only calls
   `put_unused_fd(out_fence_fd)` at line 895-896.

2. **The problem**: As noted in the commit message, `put_unused_fd()`
   doesn't free the installed file if `fd_install()` has already been
   executed. The `sync_file` structure created by `sync_file_create()`
   (line 861) contains a file reference that needs to be released with
   `fput()`.

3. **The fix**: The patch adds proper cleanup by:
   - Moving `sync_file` declaration to function scope (line 661)
   - Adding proper cleanup in the error path (lines 895-898) that calls
     both `put_unused_fd()` AND `fput(sync_file->file)` when needed

### Why This Is a Good Backport Candidate:

1. **Clear Bug Fix**: This fixes a resource leak that can accumulate
   over time as applications hit error conditions during fence
   submission.

2. **Small and Contained**: The fix is minimal - just 5 lines of actual
   code changes that properly clean up resources.

3. **No Architectural Changes**: This doesn't introduce new features or
   change any APIs - it simply fixes error handling.

4. **Similar to Previous Backported Fixes**: Looking at similar commit
   #3 ("drm/msm: Fix submit error-path leaks") which was marked for
   backporting, this follows the same pattern of fixing resource leaks
   in error paths.

5. **Low Risk**: The change only affects error paths and adds proper
   cleanup that was missing. It cannot break the normal success path.

6. **Documentation Confirms**: The sync_file.c documentation (line
   62-63) explicitly states that sync_files should be released with
   `fput(sync_file->file)`, confirming this is the correct fix.

### Pattern Analysis:

Comparing with the similar commits provided:
- Similar commits #1, #2, and #4 were NOT backported (they were general
  code improvements or non-critical changes)
- Similar commits #3 and #5 WERE backported (they fixed actual resource
  leaks or initialization failures)

This commit clearly falls into the same category as #3 and #5 - fixing
an actual resource leak that can impact system stability over time.

The leak occurs whenever:
1. A sync_file is successfully created
2. fd_install() succeeds
3. But then a subsequent error occurs before the submit completes

Without this fix, each occurrence leaks a file descriptor and associated
kernel memory, which could eventually exhaust resources in systems with
heavy GPU usage that encounter errors.

 drivers/gpu/drm/msm/msm_gem_submit.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 4ee6aeb23c512..572dd662e8095 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -724,6 +724,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	struct msm_ringbuffer *ring;
 	struct msm_submit_post_dep *post_deps = NULL;
 	struct drm_syncobj **syncobjs_to_reset = NULL;
+	struct sync_file *sync_file = NULL;
 	int out_fence_fd = -1;
 	bool has_ww_ticket = false;
 	unsigned i;
@@ -927,7 +928,7 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 	}
 
 	if (ret == 0 && args->flags & MSM_SUBMIT_FENCE_FD_OUT) {
-		struct sync_file *sync_file = sync_file_create(submit->user_fence);
+		sync_file = sync_file_create(submit->user_fence);
 		if (!sync_file) {
 			ret = -ENOMEM;
 		} else {
@@ -958,8 +959,11 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
 out_unlock:
 	mutex_unlock(&queue->lock);
 out_post_unlock:
-	if (ret && (out_fence_fd >= 0))
+	if (ret && (out_fence_fd >= 0)) {
 		put_unused_fd(out_fence_fd);
+		if (sync_file)
+			fput(sync_file->file);
+	}
 
 	if (!IS_ERR_OR_NULL(submit)) {
 		msm_gem_submit_put(submit);
-- 
2.39.5


  reply	other threads:[~2025-06-24  4:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-24  4:12 [PATCH AUTOSEL 6.1 01/15] drm/msm: Fix a fence leak in submit error path Sasha Levin
2025-06-24  4:12 ` Sasha Levin [this message]
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 03/15] ALSA: sb: Don't allow changing the DMA mode during operations Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 04/15] ALSA: sb: Force to disable DMAs once when DMA mode is changed Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 05/15] ata: libata-acpi: Do not assume 40 wire cable if no devices are enabled Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 06/15] ata: pata_cs5536: fix build on 32-bit UML Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 07/15] powerpc: Fix struct termio related ioctl macros Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 08/15] ASoC: amd: yc: update quirk data for HP Victus Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 09/15] scsi: target: Fix NULL pointer dereference in core_scsi3_decode_spec_i_port() Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 10/15] aoe: defer rexmit timer downdev work to workqueue Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 11/15] wifi: mac80211: drop invalid source address OCB frames Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 12/15] wifi: ath6kl: remove WARN on bad firmware input Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 13/15] ACPICA: Refuse to evaluate a method if arguments are missing Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 14/15] mtd: spinand: fix memory leak of ECC engine conf Sasha Levin
2025-06-24  4:12 ` [PATCH AUTOSEL 6.1 15/15] rcu: Return early if callback is not specified Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250624041238.84580-2-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=robdclark@chromium.org \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.