| Message ID | 20240606180507.3332237-1-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c8a4bdca928debacf49524d1b09dbf27e88e1f18 |
| Headers | show |
| Series | hwmon: (cros-ec_hwmon) Fix access to restricted __le16 | expand |
Thanks! On 2024-06-06 11:05:07+0000, Guenter Roeck wrote: > 0-day complains: > > drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 > > Fix by using a __le16 typed variable as parameter to le16_to_cpu(). > > Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") > Cc: Thomas Weißschuh <linux@weissschuh.net> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Acked-by: Thomas Weißschuh <linux@weissschuh.net> Guenter, does sparse work locally for you? When I run it via "make C=1 drivers/hwmon/cros_ec_hwmon.o" I only get some random warnings from included headers but nothing of value. Tzung-Bi, could you pick up this patch through chrome-platform? (And also "hwmon: (cros_ec) Prevent read overflow in probe()" [0]) [0] https://lore.kernel.org/lkml/42331b70-bd3c-496c-8c79-3ec4faad40b8@moroto.mountain/ > --- > drivers/hwmon/cros_ec_hwmon.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c > index 41f268fa8260..f586987c3502 100644 > --- a/drivers/hwmon/cros_ec_hwmon.c > +++ b/drivers/hwmon/cros_ec_hwmon.c > @@ -26,12 +26,13 @@ struct cros_ec_hwmon_priv { > static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index, u16 *speed) > { > int ret; > + __le16 __speed; > > - ret = cros_ec_cmd_readmem(cros_ec, EC_MEMMAP_FAN + index * 2, 2, speed); > + ret = cros_ec_cmd_readmem(cros_ec, EC_MEMMAP_FAN + index * 2, 2, &__speed); > if (ret < 0) > return ret; > > - *speed = le16_to_cpu(*speed); > + *speed = le16_to_cpu(__speed); > return 0; > } > > -- > 2.39.2 >
On 6/6/24 12:53, Thomas Weißschuh wrote: > Thanks! > > On 2024-06-06 11:05:07+0000, Guenter Roeck wrote: >> 0-day complains: >> >> drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 >> >> Fix by using a __le16 typed variable as parameter to le16_to_cpu(). >> >> Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") >> Cc: Thomas Weißschuh <linux@weissschuh.net> >> Cc: Tzung-Bi Shih <tzungbi@kernel.org> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > Acked-by: Thomas Weißschuh <linux@weissschuh.net> > > Guenter, does sparse work locally for you? > It does, but I use the version from git://repo.or.cz/smatch.git. Guenter
On 2024-06-06 13:16:59+0000, Guenter Roeck wrote: > On 6/6/24 12:53, Thomas Weißschuh wrote: > > Thanks! > > > > On 2024-06-06 11:05:07+0000, Guenter Roeck wrote: > > > 0-day complains: > > > > > > drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 > > > > > > Fix by using a __le16 typed variable as parameter to le16_to_cpu(). > > > > > > Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") > > > Cc: Thomas Weißschuh <linux@weissschuh.net> > > > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > > > Acked-by: Thomas Weißschuh <linux@weissschuh.net> > > > > Guenter, does sparse work locally for you? > > > > It does, but I use the version from git://repo.or.cz/smatch.git. That does indeed look much better, thanks. I have another question about the endianness conversion in general. The only places I see doing a conversion are cros_ec_sensors_cmd_read_u16() and the original cros_ec hwmon driver. Also the documentation of the EC protocol does not specify anything in that regard. Instead there is the following comment in host_event_set_bit(): /* * The overall host event implementation assumes it's running on and * communicating with little-endian architectures. */ Can the conversion be dropped?
On 6/6/24 13:48, Thomas Weißschuh wrote: > On 2024-06-06 13:16:59+0000, Guenter Roeck wrote: >> On 6/6/24 12:53, Thomas Weißschuh wrote: >>> Thanks! >>> >>> On 2024-06-06 11:05:07+0000, Guenter Roeck wrote: >>>> 0-day complains: >>>> >>>> drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 >>>> >>>> Fix by using a __le16 typed variable as parameter to le16_to_cpu(). >>>> >>>> Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") >>>> Cc: Thomas Weißschuh <linux@weissschuh.net> >>>> Cc: Tzung-Bi Shih <tzungbi@kernel.org> >>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>> >>> Acked-by: Thomas Weißschuh <linux@weissschuh.net> >>> >>> Guenter, does sparse work locally for you? >>> >> >> It does, but I use the version from git://repo.or.cz/smatch.git. > > That does indeed look much better, thanks. > > > I have another question about the endianness conversion in general. > The only places I see doing a conversion are > cros_ec_sensors_cmd_read_u16() and the original cros_ec hwmon driver. > > Also the documentation of the EC protocol does not specify anything in > that regard. > Instead there is the following comment in host_event_set_bit(): > > /* > * The overall host event implementation assumes it's running on and > * communicating with little-endian architectures. > */ > > Can the conversion be dropped? > Unless it is wrong I don't see a reason to drop it. Guenter
Hello: This patch was applied to chrome-platform/linux.git (for-kernelci) by Tzung-Bi Shih <tzungbi@kernel.org>: On Thu, 6 Jun 2024 11:05:07 -0700 you wrote: > 0-day complains: > > drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 > > Fix by using a __le16 typed variable as parameter to le16_to_cpu(). > > Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") > Cc: Thomas Weißschuh <linux@weissschuh.net> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > [...] Here is the summary with links: - hwmon: (cros-ec_hwmon) Fix access to restricted __le16 https://git.kernel.org/chrome-platform/c/c8a4bdca928d You are awesome, thank you!
Hello: This patch was applied to chrome-platform/linux.git (for-next) by Tzung-Bi Shih <tzungbi@kernel.org>: On Thu, 6 Jun 2024 11:05:07 -0700 you wrote: > 0-day complains: > > drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 > > Fix by using a __le16 typed variable as parameter to le16_to_cpu(). > > Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") > Cc: Thomas Weißschuh <linux@weissschuh.net> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > > [...] Here is the summary with links: - hwmon: (cros-ec_hwmon) Fix access to restricted __le16 https://git.kernel.org/chrome-platform/c/c8a4bdca928d You are awesome, thank you!
diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c index 41f268fa8260..f586987c3502 100644 --- a/drivers/hwmon/cros_ec_hwmon.c +++ b/drivers/hwmon/cros_ec_hwmon.c @@ -26,12 +26,13 @@ struct cros_ec_hwmon_priv { static int cros_ec_hwmon_read_fan_speed(struct cros_ec_device *cros_ec, u8 index, u16 *speed) { int ret; + __le16 __speed; - ret = cros_ec_cmd_readmem(cros_ec, EC_MEMMAP_FAN + index * 2, 2, speed); + ret = cros_ec_cmd_readmem(cros_ec, EC_MEMMAP_FAN + index * 2, 2, &__speed); if (ret < 0) return ret; - *speed = le16_to_cpu(*speed); + *speed = le16_to_cpu(__speed); return 0; }
0-day complains: drivers-hwmon-cros_ec_hwmon.c:sparse:sparse:cast-to-restricted-__le16 Fix by using a __le16 typed variable as parameter to le16_to_cpu(). Fixes: bc3e45258096 ("hwmon: add ChromeOS EC driver") Cc: Thomas Weißschuh <linux@weissschuh.net> Cc: Tzung-Bi Shih <tzungbi@kernel.org> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/hwmon/cros_ec_hwmon.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)