[bug report] hwmon: add HP WMI Sensors driver

[bug report] hwmon: add HP WMI Sensors driver

[Dan Carpenter]

Hello James Seo,

The patch 23902f98f8d4: "hwmon: add HP WMI Sensors driver" from May
22, 2023 (linux-next), leads to the following Smatch static checker
warning:

	drivers/hwmon/hp-wmi-sensors.c:1937 hp_wmi_sensors_init()
	error: uninitialized symbol 'pevents'.

drivers/hwmon/hp-wmi-sensors.c
    1913 static int hp_wmi_sensors_init(struct hp_wmi_sensors *state)
    1914 {
    1915         struct hp_wmi_info *connected[HP_WMI_MAX_INSTANCES];
    1916         struct hp_wmi_platform_events *pevents;
    1917         struct device *dev = &state->wdev->dev;
    1918         struct hp_wmi_info *info;
    1919         struct device *hwdev;
    1920         bool has_events;
    1921         bool is_new;
    1922         u8 icount;
    1923         u8 pcount;
    1924         u8 count;
    1925         int err;
    1926 
    1927         err = init_platform_events(dev, &pevents, &pcount);
    1928         if (err)
    1929                 return err;
    1930 
    1931         err = init_numeric_sensors(state, connected, &info,
    1932                                    &icount, &count, &is_new);
    1933         if (err)
    1934                 return err;
    1935 
    1936         if (IS_ENABLED(CONFIG_DEBUG_FS))
--> 1937                 hp_wmi_debugfs_init(dev, info, pevents, icount, pcount, is_new);

In this code prevents is uninitialized if pcount is zero.  It's not used
so whether this is a bug or not depends on if hp_wmi_debugfs_init() is
inlined.  If it's inlined then it's not a bug, but if it isn't then it's
still a bug (despite that pevents is not used).

    1938 
    1939         if (!count)
    1940                 return 0;        /* No connected sensors; debugfs only. */
    1941 
    1942         has_events = find_event_attributes(state, pevents, pcount);
    1943 
    1944         /* Survive failure to install WMI event handler. */
    1945         if (has_events && !add_event_handler(state))
    1946                 has_events = false;
    1947 
    1948         err = make_chip_info(state, has_events);
    1949         if (err)
    1950                 return err;
    1951 
    1952         hwdev = devm_hwmon_device_register_with_info(dev, "hp_wmi_sensors",
    1953                                                      state, &hp_wmi_chip_info,
    1954                                                      NULL);
    1955         return PTR_ERR_OR_ZERO(hwdev);
    1956 }

regards,
dan carpenter

Re: [bug report] hwmon: add HP WMI Sensors driver

[James Seo]

On Tue, Jul 25, 2023 at 08:17:50AM +0300, Dan Carpenter wrote:
> Hello James Seo,
> 
> The patch 23902f98f8d4: "hwmon: add HP WMI Sensors driver" from May
> 22, 2023 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/hwmon/hp-wmi-sensors.c:1937 hp_wmi_sensors_init()
> 	error: uninitialized symbol 'pevents'.
> 
> drivers/hwmon/hp-wmi-sensors.c
>     1913 static int hp_wmi_sensors_init(struct hp_wmi_sensors *state)
>     1914 {
>     1915         struct hp_wmi_info *connected[HP_WMI_MAX_INSTANCES];
>     1916         struct hp_wmi_platform_events *pevents;
>     1917         struct device *dev = &state->wdev->dev;
>     1918         struct hp_wmi_info *info;
>     1919         struct device *hwdev;
>     1920         bool has_events;
>     1921         bool is_new;
>     1922         u8 icount;
>     1923         u8 pcount;
>     1924         u8 count;
>     1925         int err;
>     1926 
>     1927         err = init_platform_events(dev, &pevents, &pcount);
>     1928         if (err)
>     1929                 return err;
>     1930 
>     1931         err = init_numeric_sensors(state, connected, &info,
>     1932                                    &icount, &count, &is_new);
>     1933         if (err)
>     1934                 return err;
>     1935 
>     1936         if (IS_ENABLED(CONFIG_DEBUG_FS))
> --> 1937                 hp_wmi_debugfs_init(dev, info, pevents, icount, pcount, is_new);
> 
> In this code prevents is uninitialized if pcount is zero.  It's not used
> so whether this is a bug or not depends on if hp_wmi_debugfs_init() is
> inlined.  If it's inlined then it's not a bug, but if it isn't then it's
> still a bug (despite that pevents is not used).
> 
Hi Dan,

I appreciate your report and will address this shortly.

James
> 
> regards,
> dan carpenter