[v1] staging: media: atomisp: fix map and vmap leaks in stat buffer allocation

[PATCH] staging: media: atomisp: fix map and vmap leaks in stat buffer allocation

Posted by Huihui Huang 2 months ago

Our code analyzer reported memory leaks in
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.

In atomisp_css_allocate_stat_buffers(), s3a_map is allocated by
ia_css_isp_3a_statistics_map_allocate() and its backing memory is
mapped via hmm_vmap(). When dis_buf allocation fails, the error path
frees s3a_data but does not unmap or free s3a_map. Similarly, when
md_buf allocation fails, neither s3a_map nor dvs_map (and their hmm
vmaps) are freed.

My patch adds the missing hmm_vunmap() and map free calls on both
error paths, matching the cleanup order used in
atomisp_css_free_3a_buffer() and atomisp_css_free_dis_buffer().

Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
 .../media/atomisp/pci/atomisp_compat_css20.c   | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index be5f37f4a..bfc845468 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -1116,8 +1116,12 @@ int atomisp_css_allocate_stat_buffers(struct atomisp_sub_device   *asd,
 					dvs_grid_info);
 		if (!dis_buf->dis_data) {
 			dev_err(isp->dev, "dvs buf allocation failed.\n");
-			if (s3a_buf)
+			if (s3a_buf) {
+				hmm_vunmap(s3a_buf->s3a_data->data_ptr);
+				ia_css_isp_3a_statistics_map_free(s3a_buf->s3a_map);
+				s3a_buf->s3a_map = NULL;
 				ia_css_isp_3a_statistics_free(s3a_buf->s3a_data);
+			}
 			return -EINVAL;
 		}
 
@@ -1131,10 +1135,18 @@ int atomisp_css_allocate_stat_buffers(struct atomisp_sub_device   *asd,
 		md_buf->metadata = ia_css_metadata_allocate(
 				       &asd->stream_env[stream_id].stream_info.metadata_info);
 		if (!md_buf->metadata) {
-			if (s3a_buf)
+			if (s3a_buf) {
+				hmm_vunmap(s3a_buf->s3a_data->data_ptr);
+				ia_css_isp_3a_statistics_map_free(s3a_buf->s3a_map);
+				s3a_buf->s3a_map = NULL;
 				ia_css_isp_3a_statistics_free(s3a_buf->s3a_data);
-			if (dis_buf)
+			}
+			if (dis_buf) {
+				hmm_vunmap(dis_buf->dis_data->data_ptr);
+				ia_css_isp_dvs_statistics_map_free(dis_buf->dvs_map);
+				dis_buf->dvs_map = NULL;
 				ia_css_isp_dvs2_statistics_free(dis_buf->dis_data);
+			}
 			dev_err(isp->dev, "metadata buf allocation failed.\n");
 			return -EINVAL;
 		}
-- 
2.50.1

Re: [PATCH] staging: media: atomisp: fix map and vmap leaks in stat buffer allocation

Posted by Andy Shevchenko 2 months ago

On Thu, Apr 16, 2026 at 03:27:31PM +0800, Huihui Huang wrote:
> Our code analyzer reported memory leaks in
> drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.
> 
> In atomisp_css_allocate_stat_buffers(), s3a_map is allocated by
> ia_css_isp_3a_statistics_map_allocate() and its backing memory is
> mapped via hmm_vmap(). When dis_buf allocation fails, the error path
> frees s3a_data but does not unmap or free s3a_map. Similarly, when
> md_buf allocation fails, neither s3a_map nor dvs_map (and their hmm
> vmaps) are freed.

> My patch adds the missing hmm_vunmap() and map free calls on both
> error paths, matching the cleanup order used in
> atomisp_css_free_3a_buffer() and atomisp_css_free_dis_buffer().

Imperative mood.

...

>  			dev_err(isp->dev, "dvs buf allocation failed.\n");
> -			if (s3a_buf)
> +			if (s3a_buf) {
> +				hmm_vunmap(s3a_buf->s3a_data->data_ptr);
> +				ia_css_isp_3a_statistics_map_free(s3a_buf->s3a_map);
> +				s3a_buf->s3a_map = NULL;

Are these NULLifications needed? It sounds like it tries to paper over some
potential UAF cases. Is there any possibility to access s3a_map at this point?

>  				ia_css_isp_3a_statistics_free(s3a_buf->s3a_data);
> +			}

-- 
With Best Regards,
Andy Shevchenko

[PATCH v2] staging: media: atomisp: fix map and vmap leaks in stat buffer allocation

Posted by Huihui Huang 2 months ago

There are memory leaks in
drivers/staging/media/atomisp/pci/atomisp_compat_css20.c.

In atomisp_css_allocate_stat_buffers(), s3a_map is allocated by
ia_css_isp_3a_statistics_map_allocate() and its backing memory is
mapped via hmm_vmap(). When dis_buf allocation fails, the error path
frees s3a_data but does not unmap or free s3a_map. Similarly, when
md_buf allocation fails, neither s3a_map nor dvs_map (and their hmm
vmaps) are freed.

Add the missing hmm_vunmap() and map free calls on both error paths,
matching the cleanup order used in atomisp_css_free_3a_buffer() and
atomisp_css_free_dis_buffer().

Signed-off-by: Huihui Huang <hhhuang@smu.edu.sg>
---
v2: Reword commit message per review feedback. Remove unnecessary
    NULL assignments on error paths.
---
 .../media/atomisp/pci/atomisp_compat_css20.c      | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
index be5f37f4a6fd..27e6f6563f14 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_compat_css20.c
@@ -1116,8 +1116,11 @@ int atomisp_css_allocate_stat_buffers(struct atomisp_sub_device   *asd,
 					dvs_grid_info);
 		if (!dis_buf->dis_data) {
 			dev_err(isp->dev, "dvs buf allocation failed.\n");
-			if (s3a_buf)
+			if (s3a_buf) {
+				hmm_vunmap(s3a_buf->s3a_data->data_ptr);
+				ia_css_isp_3a_statistics_map_free(s3a_buf->s3a_map);
 				ia_css_isp_3a_statistics_free(s3a_buf->s3a_data);
+			}
 			return -EINVAL;
 		}
 
@@ -1131,10 +1134,16 @@ int atomisp_css_allocate_stat_buffers(struct atomisp_sub_device   *asd,
 		md_buf->metadata = ia_css_metadata_allocate(
 				       &asd->stream_env[stream_id].stream_info.metadata_info);
 		if (!md_buf->metadata) {
-			if (s3a_buf)
+			if (s3a_buf) {
+				hmm_vunmap(s3a_buf->s3a_data->data_ptr);
+				ia_css_isp_3a_statistics_map_free(s3a_buf->s3a_map);
 				ia_css_isp_3a_statistics_free(s3a_buf->s3a_data);
-			if (dis_buf)
+			}
+			if (dis_buf) {
+				hmm_vunmap(dis_buf->dis_data->data_ptr);
+				ia_css_isp_dvs_statistics_map_free(dis_buf->dvs_map);
 				ia_css_isp_dvs2_statistics_free(dis_buf->dis_data);
+			}
 			dev_err(isp->dev, "metadata buf allocation failed.\n");
 			return -EINVAL;
 		}
-- 
2.50.1