linux-kernel - Re: [PATCH 5/6] media: staging: media: imx6-mipi-csi2: use guard() to simplify code

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <90e5e430743eaca4b21097c64c42dd4f579bc48b.camel@pengutronix.de>
Date: Mon, 10 Nov 2025 09:27:21 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Frank Li <Frank.Li@....com>, Steve Longerbeam <slongerbeam@...il.com>, 
 Mauro Carvalho Chehab	 <mchehab@...nel.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, Shawn Guo <shawnguo@...nel.org>, Sascha Hauer
 <s.hauer@...gutronix.de>, Pengutronix Kernel Team	 <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>
Cc: imx@...ts.linux.dev, linux-media@...r.kernel.org, 
	linux-staging@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/6] media: staging: media: imx6-mipi-csi2: use guard()
 to simplify code

On Fr, 2025-11-07 at 15:44 -0500, Frank Li wrote:
> Use guard() to simplify mutex locking. No functional change.
> 
> Signed-off-by: Frank Li <Frank.Li@....com>
> ---
>  drivers/staging/media/imx/imx6-mipi-csi2.c | 54 +++++++++++-------------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx6-mipi-csi2.c b/drivers/staging/media/imx/imx6-mipi-csi2.c
> index f8d0b3b8b250774af51f2aba731e639131aceff9..54e9491d3428686288a5bc9bb58a5a0a25aca696 100644
> --- a/drivers/staging/media/imx/imx6-mipi-csi2.c
> +++ b/drivers/staging/media/imx/imx6-mipi-csi2.c
> @@ -413,21 +413,17 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	struct csi2_dev *csi2 = sd_to_dev(sd);
>  	int i, ret = 0;
>  
> -	mutex_lock(&csi2->lock);
> +	guard(mutex)(&csi2->lock);
>  
> -	if (!csi2->src_sd) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (!csi2->src_sd)
> +		return -EPIPE;
>  
>  	for (i = 0; i < CSI2_NUM_SRC_PADS; i++) {
>  		if (csi2->sink_linked[i])
>  			break;
>  	}
> -	if (i >= CSI2_NUM_SRC_PADS) {
> -		ret = -EPIPE;
> -		goto out;
> -	}
> +	if (i >= CSI2_NUM_SRC_PADS)
> +		return -EPIPE;
>  
>  	/*
>  	 * enable/disable streaming only if stream_count is
> @@ -442,14 +438,12 @@ static int csi2_s_stream(struct v4l2_subdev *sd, int enable)
>  	else
>  		csi2_stop(csi2);
>  	if (ret)
> -		goto out;
> +		return ret;
>  
>  update_count:
>  	csi2->stream_count += enable ? 1 : -1;
>  	if (csi2->stream_count < 0)
>  		csi2->stream_count = 0;
> -out:
> -	mutex_unlock(&csi2->lock);
>  	return ret;

This is mixing goto and cleanup in the same function, which the scope-
based cleanup helpers documentation suggests shouldn't be done.

In this case I see no problem because we only jump to update_count from
under the lock, but since issues with gotos around automatic cleanup
are easy to miss, I think it would be better to also turn the
update_count jump into a conditional block.


regards
Philipp