linux-kernel - Re: [PATCH 1/1] et8ek8: Decrease stack usage

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-next>] [day] [month] [year] [list]

Date:   Wed, 16 Aug 2017 10:13:05 +0200
From:   Pavel Machek <pavel@....cz>
To:     Sakari Ailus <sakari.ailus@...ux.intel.com>
Cc:     linux-media@...r.kernel.org,
        kernel list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] et8ek8: Decrease stack usage

On Wed 2017-08-16 10:33:45, Sakari Ailus wrote:
> The et8ek8 driver combines I²C register writes to a single array that it
> passes to i2c_transfer(). The maximum number of writes is 48 at once,
> decrease it to 8 and make more transfers if needed, thus avoiding a
> warning on stack usage.

Dunno. Slowing down code to save stack does not sound attractive.

What about this one? Way simpler, too... (Unless there's some rule
about i2c, DMA and static buffers. Is it?)

Signed-off-by: Pavel Machek <pavel@....cz>

 (untested)
								Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index f39f517..64da731 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -227,7 +227,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
 					  int cnt)
 {
 	struct i2c_msg msg[ET8EK8_MAX_MSG];
-	unsigned char data[ET8EK8_MAX_MSG][6];
+	static unsigned char data[ET8EK8_MAX_MSG][6];
 	int wcnt = 0;
 	u16 reg, data_length;
 	u32 val;



> ---
> Pavel: this is just compile tested. Could you test it on N900, please?
> 
>  drivers/media/i2c/et8ek8/et8ek8_driver.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> index f39f517..c14f0fd 100644
> --- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
> +++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
> @@ -43,7 +43,7 @@
>  
>  #define ET8EK8_NAME		"et8ek8"
>  #define ET8EK8_PRIV_MEM_SIZE	128
> -#define ET8EK8_MAX_MSG		48
> +#define ET8EK8_MAX_MSG		8
>  
>  struct et8ek8_sensor {
>  	struct v4l2_subdev subdev;
> @@ -220,7 +220,8 @@ static void et8ek8_i2c_create_msg(struct i2c_client *client, u16 len, u16 reg,
>  
>  /*
>   * A buffered write method that puts the wanted register write
> - * commands in a message list and passes the list to the i2c framework
> + * commands in smaller number of message lists and passes the lists to
> + * the i2c framework
>   */
>  static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
>  					  const struct et8ek8_reg *wnext,
> @@ -231,11 +232,7 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
>  	int wcnt = 0;
>  	u16 reg, data_length;
>  	u32 val;
> -
> -	if (WARN_ONCE(cnt > ET8EK8_MAX_MSG,
> -		      ET8EK8_NAME ": %s: too many messages.\n", __func__)) {
> -		return -EINVAL;
> -	}
> +	int rval;
>  
>  	/* Create new write messages for all writes */
>  	while (wcnt < cnt) {
> @@ -249,10 +246,21 @@ static int et8ek8_i2c_buffered_write_regs(struct i2c_client *client,
>  
>  		/* Update write count */
>  		wcnt++;
> +
> +		if (wcnt < ET8EK8_MAX_MSG)
> +			continue;
> +
> +		rval = i2c_transfer(client->adapter, msg, wcnt);
> +		if (rval < 0)
> +			return rval;
> +
> +		cnt -= wcnt;
> +		wcnt = 0;
>  	}
>  
> -	/* Now we send everything ... */
> -	return i2c_transfer(client->adapter, msg, wcnt);
> +	rval = i2c_transfer(client->adapter, msg, wcnt);
> +
> +	return rval < 0 ? rval : 0;
>  }
>  
>  /*

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)