Re: [PATCH] media: atomisp: fix CAS scaler descriptor leaks

[Dan Carpenter]

On Mon, Jun 29, 2026 at 02:16:21PM +0300, Andy Shevchenko wrote:
> On Mon, Jun 29, 2026 at 11:30:40AM +0300, Dan Carpenter wrote:
> > On Sat, Jun 27, 2026 at 02:01:51PM +0800, Dawei Feng wrote:
> 
> ...
> 
> > free_output_stage:
> > 	if (need_scalar) {
> > 		kfree(mycs->is_output_stage);
> > 		mycs->is_output_stage = NULL;
> > 	}
> > free_scalar_binary:
> > 	if (need_scalar) {
> > 		kfree(mycs->yuv_scaler_binary);
> > 		mycs->yuv_scaler_binary = NULL;
> > 	}
> 
> If we go this way, double check that the checks are needed as we have kfree()
> to be NULL-aware.

Dawei, Andy is the one reviewing atomisp so his opinion matters more
than mine here.  So do what he says.

But I don't really agree...

In this case, sure, hopefully the caller zeroes the ->yuv_scaler_binary
pointer, but if we just follow the simple rule of only undoing things
which we have done then we don't need to check.  The function is
self contained and self explanatory.

And more generally, I've always hated patches which delete NULL
checks before a ionmap() or whatever.  Hiding the NULL check inside the
free function makes the code less self contained.  The real fix is to
stop mixing allocated and unallocated pointers.  Then you don't need a
NULL check because you already know.  (Also I think those iounmap()
patches were wrong because some arches have a warning when you unmap
a NULL).

regards,
dan carpenter