linux-kernel - RE: [PATCH] spi: aspeed: Fix an error handling path in aspeed_spi_[read|write]

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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <TYZPR06MB5203E218B914FDFCAA0E384EB2212@TYZPR06MB5203.apcprd06.prod.outlook.com>
Date: Wed, 20 Nov 2024 02:08:41 +0000
From: Chin-Ting Kuo <chin-ting_kuo@...eedtech.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	Cédric Le Goater <clg@...d.org>, Mark Brown
	<broonie@...nel.org>, Joel Stanley <joel@....id.au>, Andrew Jeffery
	<andrew@...econstruct.com.au>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"kernel-janitors@...r.kernel.org" <kernel-janitors@...r.kernel.org>,
	"linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
	"openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: RE: [PATCH] spi: aspeed: Fix an error handling path in
 aspeed_spi_[read|write]_user()

Hi Christophe,

Thanks for the patch.

> -----Original Message-----
> From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> Sent: Wednesday, November 20, 2024 5:30 AM
> Subject: [PATCH] spi: aspeed: Fix an error handling path in
> aspeed_spi_[read|write]_user()
> 
> A aspeed_spi_start_user() is not balanced by a corresponding
> aspeed_spi_stop_user().
> Add the missing call.
> 
> Fixes: e3228ed92893 ("spi: spi-mem: Convert Aspeed SMC driver to spi-mem")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> Compile tested only.
> 
> 
> This patch is completely speculative, review with care!
> 
> It is only based on naming where a _start() function if not followed by a
> _stop() in some paths but is in other paths.
> ---
>  drivers/spi/spi-aspeed-smc.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/spi/spi-aspeed-smc.c b/drivers/spi/spi-aspeed-smc.c index
> 8eb843ddb25f..e9beae95dded 100644
> --- a/drivers/spi/spi-aspeed-smc.c
> +++ b/drivers/spi/spi-aspeed-smc.c
> @@ -239,7 +239,7 @@ static ssize_t aspeed_spi_read_user(struct
> aspeed_spi_chip *chip,
> 
>  	ret = aspeed_spi_send_cmd_addr(chip, op->addr.nbytes, offset,
> op->cmd.opcode);
>  	if (ret < 0)
> -		return ret;
> +		goto stop_user;
> 
>  	if (op->dummy.buswidth && op->dummy.nbytes) {
>  		for (i = 0; i < op->dummy.nbytes / op->dummy.buswidth; i++) @@
> -249,8 +249,9 @@ static ssize_t aspeed_spi_read_user(struct aspeed_spi_chip
> *chip,
>  	aspeed_spi_set_io_mode(chip, io_mode);
> 
>  	aspeed_spi_read_from_ahb(buf, chip->ahb_base, len);

How about adding a blank line here?

> +stop_user:
>  	aspeed_spi_stop_user(chip);
> -	return 0;
> +	return ret;
>  }
> 
>  static ssize_t aspeed_spi_write_user(struct aspeed_spi_chip *chip, @@
> -261,10 +262,11 @@ static ssize_t aspeed_spi_write_user(struct
> aspeed_spi_chip *chip,
>  	aspeed_spi_start_user(chip);
>  	ret = aspeed_spi_send_cmd_addr(chip, op->addr.nbytes, op->addr.val,
> op->cmd.opcode);
>  	if (ret < 0)
> -		return ret;
> +		goto stop_user;
>  	aspeed_spi_write_to_ahb(chip->ahb_base, op->data.buf.out,
> op->data.nbytes);

A blank line here?

> +stop_user:
>  	aspeed_spi_stop_user(chip);
> -	return 0;
> +	return ret;
>  }
> 
>  /* support for 1-1-1, 1-1-2 or 1-1-4 */
> --
> 2.47.0

Chin-Ting