public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] media: atomisp: Fix memory leak in atomisp_fixed_pattern_table()
@ 2026-02-03 13:56 Zilin Guan
  2026-02-03 15:08 ` Andy Shevchenko
  0 siblings, 1 reply; 3+ messages in thread
From: Zilin Guan @ 2026-02-03 13:56 UTC (permalink / raw)
  To: andy.shevchenko
  Cc: andy, gregkh, hansg, hverkuil, jianhao.xu, linux-kernel,
	linux-media, linux-staging, mchehab, sakari.ailus, zilin

atomisp_v4l2_framebuffer_to_css_frame() allocates memory for
temporary variable raw_black_frame, which must be released via
ia_css_frame_free() before the function returns. However, if
sh_css_set_black_frame() fails, the function returns immediately without
performing this cleanup, leading to a memory leak.

Fix this by assigning the return value of sh_css_set_black_frame() to
ret. This ensures that the error code is propagated while allowing the
execution to fall through to the ia_css_frame_free() cleanup call.

The bug was originally detected on v6.13-rc1 using an experimental
static analysis tool we are developing, and we have verified that the
issue persists in the latest mainline kernel. The tool is based on the
LLVM framework and is specifically designed to detect memory management
issues. It is currently under active development and not yet publicly
available.

We performed build testing on x86_64 with allyesconfig. Since triggering
this error path in atomisp requires specific Intel Atom ISP hardware and
firmware, we were unable to perform runtime testing and instead verified
the fix according to the code logic.

Fixes: 85b606e02ad7 ("media: atomisp: get rid of a bunch of other wrappers")
Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
Changes in v3:
- Assign the result of sh_css_set_black_frame() to ret instead of
  hardcoding -ENOMEM.

Changes in v2:
- Add detailed explanation about the tool and verification method.

 drivers/staging/media/atomisp/pci/atomisp_cmd.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
index 3a4eb4f6d3be..ae9c4dc8a44b 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_cmd.c
@@ -3367,10 +3367,8 @@ int atomisp_fixed_pattern_table(struct atomisp_sub_device *asd,
 	if (ret)
 		return ret;
 
-	if (sh_css_set_black_frame(asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].stream,
-				   raw_black_frame) != 0)
-		return -ENOMEM;
-
+	ret = sh_css_set_black_frame(asd->stream_env[ATOMISP_INPUT_STREAM_GENERAL].stream,
+				     raw_black_frame);
 	ia_css_frame_free(raw_black_frame);
 	return ret;
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] media: atomisp: Fix memory leak in atomisp_fixed_pattern_table()
  2026-02-03 13:56 [PATCH v3] media: atomisp: Fix memory leak in atomisp_fixed_pattern_table() Zilin Guan
@ 2026-02-03 15:08 ` Andy Shevchenko
  2026-02-03 16:21   ` Zilin Guan
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2026-02-03 15:08 UTC (permalink / raw)
  To: Zilin Guan
  Cc: andy.shevchenko, andy, gregkh, hansg, hverkuil, jianhao.xu,
	linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus

On Tue, Feb 03, 2026 at 01:56:59PM +0000, Zilin Guan wrote:
> atomisp_v4l2_framebuffer_to_css_frame() allocates memory for
> temporary variable raw_black_frame, which must be released via
> ia_css_frame_free() before the function returns. However, if
> sh_css_set_black_frame() fails, the function returns immediately without
> performing this cleanup, leading to a memory leak.
> 
> Fix this by assigning the return value of sh_css_set_black_frame() to
> ret. This ensures that the error code is propagated while allowing the
> execution to fall through to the ia_css_frame_free() cleanup call.
> 
> The bug was originally detected on v6.13-rc1 using an experimental
> static analysis tool we are developing, and we have verified that the
> issue persists in the latest mainline kernel. The tool is based on the
> LLVM framework and is specifically designed to detect memory management
> issues. It is currently under active development and not yet publicly
> available.
> 
> We performed build testing on x86_64 with allyesconfig. Since triggering
> this error path in atomisp requires specific Intel Atom ISP hardware and
> firmware, we were unable to perform runtime testing and instead verified
> the fix according to the code logic.

> Fixes: 85b606e02ad7 ("media: atomisp: get rid of a bunch of other wrappers")
> Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>

I haven't suggested this patch. I only provided a hint on a better
implementation, so there are no grounds to have this tag to be here.

Replace that with

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
as current version LGTM.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v3] media: atomisp: Fix memory leak in atomisp_fixed_pattern_table()
  2026-02-03 15:08 ` Andy Shevchenko
@ 2026-02-03 16:21   ` Zilin Guan
  0 siblings, 0 replies; 3+ messages in thread
From: Zilin Guan @ 2026-02-03 16:21 UTC (permalink / raw)
  To: andriy.shevchenko
  Cc: andy.shevchenko, andy, gregkh, hansg, hverkuil, jianhao.xu,
	linux-kernel, linux-media, linux-staging, mchehab, sakari.ailus,
	zilin

On Tue, Feb 03, 2026 at 05:08:51PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 03, 2026 at 01:56:59PM +0000, Zilin Guan wrote:
> > atomisp_v4l2_framebuffer_to_css_frame() allocates memory for
> > temporary variable raw_black_frame, which must be released via
> > ia_css_frame_free() before the function returns. However, if
> > sh_css_set_black_frame() fails, the function returns immediately without
> > performing this cleanup, leading to a memory leak.
> > 
> > Fix this by assigning the return value of sh_css_set_black_frame() to
> > ret. This ensures that the error code is propagated while allowing the
> > execution to fall through to the ia_css_frame_free() cleanup call.
> > 
> > The bug was originally detected on v6.13-rc1 using an experimental
> > static analysis tool we are developing, and we have verified that the
> > issue persists in the latest mainline kernel. The tool is based on the
> > LLVM framework and is specifically designed to detect memory management
> > issues. It is currently under active development and not yet publicly
> > available.
> > 
> > We performed build testing on x86_64 with allyesconfig. Since triggering
> > this error path in atomisp requires specific Intel Atom ISP hardware and
> > firmware, we were unable to perform runtime testing and instead verified
> > the fix according to the code logic.
> 
> > Fixes: 85b606e02ad7 ("media: atomisp: get rid of a bunch of other wrappers")
> > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> I haven't suggested this patch. I only provided a hint on a better
> implementation, so there are no grounds to have this tag to be here.
> 
> Replace that with
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> as current version LGTM.
> 
> -- 
> With Best Regards,
> Andy Shevchenko

Hi Andy,

Thank you for the review and clarification.

I apologize for the misunderstanding regarding the Suggested-by tag. 
I will remove it and apply your Reviewed-by tag in the next version.

Best regards,
Zilin Guan

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-02-03 16:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-03 13:56 [PATCH v3] media: atomisp: Fix memory leak in atomisp_fixed_pattern_table() Zilin Guan
2026-02-03 15:08 ` Andy Shevchenko
2026-02-03 16:21   ` Zilin Guan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox