diff mbox series

thermal/drivers/mediatek/lvts_thermal: fix memcpy's number of bytes in lvts_calibration_init()

Message ID 20230307134245.83599-1-bchihi@baylibre.com
State New
Headers
Series thermal/drivers/mediatek/lvts_thermal: fix memcpy's number of bytes in lvts_calibration_init() |

Commit Message

Balsam CHIHI March 7, 2023, 1:42 p.m. UTC 
  From: Balsam CHIHI <bchihi@baylibre.com>

Replace memcpy 2 bytes by sizeof(int) bytes of LVTS calibration data.

Reported-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: Balsam CHIHI <bchihi@baylibre.com>
---
Rebased on top of thermal/linux-next
base-commit: 6828e402d06f7c574430b61c05db784cd847b19f

Original email :
Hello Balsam CHIHI,

The patch f5f633b18234: "thermal/drivers/mediatek: Add the Low
Voltage Thermal Sensor driver" from Feb 9, 2023, leads to the
following Smatch static checker warning:

        drivers/thermal/mediatek/lvts_thermal.c:562 lvts_calibration_init()
        warn: not copying enough bytes for '&lvts_ctrl->calibration[i]' (4 vs 2 bytes)

drivers/thermal/mediatek/lvts_thermal.c
    555 static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl,
    556                                         const struct lvts_ctrl_data *lvts_ctrl_data,
    557                                         u8 *efuse_calibration)
    558 {
    559         int i;
    560
    561         for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++)
--> 562                 memcpy(&lvts_ctrl->calibration[i],
    563                        efuse_calibration + lvts_ctrl_data->cal_offset[i], 2);
                                                                                  ^
This is copying an array of known ints to a u32 array.  It should copy
sizeof(int) instead of 2.  It only works because the data you're on
little endian and the data is small.

    564
    565         return 0;
    566 }

regards,
dan carpenter
---
---
 drivers/thermal/mediatek/lvts_thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 6828e402d06f7c574430b61c05db784cd847b19f
prerequisite-patch-id: 73be949bd16979769e5b94905b244dcee4a8f687
prerequisite-patch-id: d23d83a946e5b876ef01a717fd51b07df1fa08dd
prerequisite-patch-id: d67f2455eef1c4a9ecc460dbf3c2e3ad47d213ec
prerequisite-patch-id: 9076e9b3bd3cc411b7b80344211364db5f0cca17
prerequisite-patch-id: e220d6ae26786f524c249588433f02e5f5f906ad
prerequisite-patch-id: b407d2998e57678952128b3a4bac92a379132b09
prerequisite-patch-id: fbb9212ce8c3530da17d213f56fa334ce4fa1b2b
prerequisite-patch-id: 5db9eed2659028cf4419f2de3d093af7df6c2dad
prerequisite-patch-id: a83c00c628605d1c8fbe1d97074f9f28efb1bcfc
prerequisite-patch-id: 56a245620a4f8238cf1ba3844dc348de3db33845
prerequisite-patch-id: 7df24b0bf11129ddd3356eacf192cc3fdb2bcded
prerequisite-patch-id: 3213ca70cb5b26d54a7137ff40ca8cd2a795c414
prerequisite-patch-id: 6c2202e85215d1c7e8ab16a6b85922e994c68d9b

Comments

AngeloGioacchino Del Regno March 8, 2023, 9:10 a.m. UTC | #1 
Il 07/03/23 14:42, bchihi@baylibre.com ha scritto:
> From: Balsam CHIHI <bchihi@baylibre.com>
> 
> Replace memcpy 2 bytes by sizeof(int) bytes of LVTS calibration data.

sizeof(int) is architecture dependant... please use a fixed size type instead.

Also, shouldn't this be u16?!

Regards,
Angelo
Dan Carpenter March 9, 2023, 12:37 p.m. UTC | #2 
On Wed, Mar 08, 2023 at 10:10:34AM +0100, AngeloGioacchino Del Regno wrote:
> Il 07/03/23 14:42, bchihi@baylibre.com ha scritto:
> > From: Balsam CHIHI <bchihi@baylibre.com>
> > 
> > Replace memcpy 2 bytes by sizeof(int) bytes of LVTS calibration data.
> 
> sizeof(int) is architecture dependant...
> 

On Linux sizeof(int) is always 4.

I'm just so confused what you are talking about.  Are you thinking about
sizeof(long)?  Are you thinking about CPUs from the 1970s?  Linux wasn't
invented until the 90s so the old CPUs were already in museums at that
point.

> please use a fixed size type instead.

This is an unusual style opinion that I have not heard before.
Hopefully, you just got ints and longs confused so we can move on
without discussing it too much.  We're copying an int so sizeof(int) is
obviously correct.  It's hard to know how to respond.

> Also, shouldn't this be u16?!

What? Why would you think that?

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/thermal/mediatek/lvts_thermal.c b/drivers/thermal/mediatek/lvts_thermal.c
index ddfdcbcf6d86..b505c6b49031 100644
--- a/drivers/thermal/mediatek/lvts_thermal.c
+++ b/drivers/thermal/mediatek/lvts_thermal.c
@@ -575,7 +575,7 @@  static int lvts_calibration_init(struct device *dev, struct lvts_ctrl *lvts_ctrl
 
 	for (i = 0; i < lvts_ctrl_data->num_lvts_sensor; i++)
 		memcpy(&lvts_ctrl->calibration[i],
-		       efuse_calibration + lvts_ctrl_data->cal_offset[i], 2);
+		       efuse_calibration + lvts_ctrl_data->cal_offset[i], sizeof(int));
 
 	return 0;
 }