Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1611651
| 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 |
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 | Next — Previous in thread | Next in thread | Find similar | Unroll 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