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 <a...@arndb.de>
Reviewed-by: Dmitry Torokhov <dmitry.torok...@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
