Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1611651

Re: [PATCH] auxdisplay: ht16k33: don't access uninitialized data

From Dmitry Torokhov <dmitry.torokhov@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH] auxdisplay: ht16k33: don't access uninitialized data
Date 2017-03-29 09:40 +0200
Message-ID <tqgmZ-3Eh-9@gated-at.bofh.it> (permalink)
References <tpWoi-65Z-23@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, Mar 28, 2017 at 12:11:49PM +0200, Arnd Bergmann wrote:
> gcc-7.0.1 points out that we copy uninitialized data from the stack
> into a per-device structure:
> 
> drivers/auxdisplay/ht16k33.c: In function 'ht16k33_keypad_irq_thread':
> arch/x86/include/asm/string_32.h:78:16: error: 'new_state' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> arch/x86/include/asm/string_32.h:79:22: error: '*((void *)&new_state+4)' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> 
> The access is harmless because we never read the data, but we are better
> off not doing this, so this changes the code to only copy the data
> that was actually initialized. To make sure we don't overflow the
> stack with an incorrect DT, we also need to add a sanity checkin the
> probe function.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
>  drivers/auxdisplay/ht16k33.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/auxdisplay/ht16k33.c b/drivers/auxdisplay/ht16k33.c
> index f66b45b235b0..ba6370974574 100644
> --- a/drivers/auxdisplay/ht16k33.c
> +++ b/drivers/auxdisplay/ht16k33.c
> @@ -278,7 +278,7 @@ static bool ht16k33_keypad_scan(struct ht16k33_keypad *keypad)
>  		}
>  	}
>  	input_sync(keypad->dev);
> -	memcpy(keypad->last_key_state, new_state, sizeof(new_state));
> +	memcpy(keypad->last_key_state, new_state, sizeof(u16) * keypad->cols);
>  
>  	return pressed;
>  }
> @@ -353,6 +353,12 @@ static int ht16k33_keypad_probe(struct i2c_client *client,
>  	err = matrix_keypad_parse_of_params(&client->dev, &rows, &cols);
>  	if (err)
>  		return err;
> +	if (rows > HT16K33_MATRIX_KEYPAD_MAX_ROWS ||
> +	    cols > HT16K33_MATRIX_KEYPAD_MAX_COLS) {
> +		dev_err(&client->dev, "%u rows or %u cols out of range in DT\n",
> +			rows, cols);
> +		return -ERANGE;
> +	}
>  
>  	keypad->rows = rows;
>  	keypad->cols = cols;
> -- 
> 2.9.0
> 

-- 
Dmitry

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] auxdisplay: ht16k33: don't access uninitialized data Arnd Bergmann <arnd@arndb.de> - 2017-03-28 12:20 +0200
  Re: [PATCH] auxdisplay: ht16k33: don't access uninitialized data Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-03-29 09:40 +0200
  Re: [PATCH] auxdisplay: ht16k33: don't access uninitialized data Robin van der Gracht <robin@protonic.nl> - 2017-03-29 10:10 +0200

csiph-web