2016-12-15 04:14:31

by Darren Hart

[permalink] [raw]
Subject: [PATCH v2] platform/x86: thinkpad_acpi: Initialize local in_tablet_mode and type

linux-next reported in_tablet_mode and type may be used uninitialized
after:

b31800283868 ("platform/x86: thinkpad_acpi: Move tablet detection into separate function")

This turns out to be a false positive as the pr_info call cannot be
reached if tp_features.hotkey_tablet (global scope) is 0, and
in_tablet_mode and type are assigned in both places
tp_features.hotkey_tablet is assigned.

Regardless, to make it explicit and avoid further reports, initialize
in_tablet_mode to 0 and type to "".

Signed-off-by: Darren Hart <[email protected]>
Cc: Lyude <[email protected]>
Cc: Henrique de Moraes Holschuh <[email protected]>
Cc: Andy Shevchenko <[email protected]>
---
Since v1: Initialize type also.

drivers/platform/x86/thinkpad_acpi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 69cb0da..c408390 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -3143,8 +3143,8 @@ typedef tpacpi_keymap_entry_t tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];

static int hotkey_init_tablet_mode(void)
{
- int in_tablet_mode, res;
- char *type;
+ int in_tablet_mode = 0, res;
+ char *type = "";

if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
/* For X41t, X60t, X61t Tablets... */
--
2.9.3


--
Darren Hart
Intel Open Source Technology Center


Subject: Re: [PATCH v2] platform/x86: thinkpad_acpi: Initialize local in_tablet_mode and type

On Wed, 14 Dec 2016, Darren Hart wrote:
> linux-next reported in_tablet_mode and type may be used uninitialized
> after:
>
> b31800283868 ("platform/x86: thinkpad_acpi: Move tablet detection into separate function")
>
> This turns out to be a false positive as the pr_info call cannot be
> reached if tp_features.hotkey_tablet (global scope) is 0, and
> in_tablet_mode and type are assigned in both places
> tp_features.hotkey_tablet is assigned.
>
> Regardless, to make it explicit and avoid further reports, initialize
> in_tablet_mode to 0 and type to "".
>
> Signed-off-by: Darren Hart <[email protected]>
> Cc: Lyude <[email protected]>
> Cc: Henrique de Moraes Holschuh <[email protected]>
> Cc: Andy Shevchenko <[email protected]>

Acked-by: Henrique de Moraes Holschuh <[email protected]>

> ---
> Since v1: Initialize type also.
>
> drivers/platform/x86/thinkpad_acpi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 69cb0da..c408390 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -3143,8 +3143,8 @@ typedef tpacpi_keymap_entry_t tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
>
> static int hotkey_init_tablet_mode(void)
> {
> - int in_tablet_mode, res;
> - char *type;
> + int in_tablet_mode = 0, res;
> + char *type = "";
>
> if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
> /* For X41t, X60t, X61t Tablets... */
> --
> 2.9.3
>
>

--
Henrique Holschuh

"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot

2016-12-15 18:15:07

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: thinkpad_acpi: Initialize local in_tablet_mode and type

On Wed, 2016-12-14 at 20:14 -0800, Darren Hart wrote:
> linux-next reported in_tablet_mode and type may be used uninitialized
> after:
>
> b31800283868 ("platform/x86: thinkpad_acpi: Move tablet detection into
> separate function")
>
> This turns out to be a false positive as the pr_info call cannot be
> reached if tp_features.hotkey_tablet (global scope) is 0, and
> in_tablet_mode and type are assigned in both places
> tp_features.hotkey_tablet is assigned.
>
> Regardless, to make it explicit and avoid further reports, initialize
> in_tablet_mode to 0 and type to "".
>

@@ -3143,8 +3143,8 @@ typedef tpacpi_keymap_entry_t
> tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
>  
>  static int hotkey_init_tablet_mode(void)
>  {
> - int in_tablet_mode, res;
> - char *type;
> + int in_tablet_mode = 0, res;
> + char *type = "";

Subtle correction
NULL will work either and takes less memory ;)

P.S. Even pr_*() is NULL-aware


>  
>   if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
>   /* For X41t, X60t, X61t Tablets... */
> -- 
> 2.9.3
>
>

--
Andy Shevchenko <[email protected]>
Intel Finland Oy

2016-12-15 21:17:14

by Darren Hart

[permalink] [raw]
Subject: Re: [PATCH v2] platform/x86: thinkpad_acpi: Initialize local in_tablet_mode and type

On Thu, Dec 15, 2016 at 08:15:02PM +0200, Andy Shevchenko wrote:
> On Wed, 2016-12-14 at 20:14 -0800, Darren Hart wrote:
> > linux-next reported in_tablet_mode and type may be used uninitialized
> > after:
> >
> > b31800283868 ("platform/x86: thinkpad_acpi: Move tablet detection into
> > separate function")
> >
> > This turns out to be a false positive as the pr_info call cannot be
> > reached if tp_features.hotkey_tablet (global scope) is 0, and
> > in_tablet_mode and type are assigned in both places
> > tp_features.hotkey_tablet is assigned.
> >
> > Regardless, to make it explicit and avoid further reports, initialize
> > in_tablet_mode to 0 and type to "".
> >
>
> @@ -3143,8 +3143,8 @@ typedef tpacpi_keymap_entry_t
> > tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
> > ?
> > ?static int hotkey_init_tablet_mode(void)
> > ?{
> > - int in_tablet_mode, res;
> > - char *type;
> > + int in_tablet_mode = 0, res;
> > + char *type = "";
>
> Subtle correction
> NULL will work either and takes less memory ;)
>
> P.S. Even pr_*() is NULL-aware

Agreed. Fixed, pushed, tagged. Thanks all.

--
Darren Hart
Intel Open Source Technology Center