There is a serious bug in regmap_read() and regmap_write() functions
where an uint pointer is cast to (void *) which is then cast to (u8 *),
(u16 *), (u32 *) or (u64 *), depending on register width of the map.

For example given a regmap with 16-bit register width the code
        int val = 0x12340000;
        regmap_read(map, 0, &val);
only changes the lower 16 bits of val on little-endian machines.
The upper 16 bits will remain 0x1234.

Nobody noticed this probably because this bug can be triggered with
regmap_write() only on big-endian architectures (which are not used by
many people anymore), and on little endian this bug has consequences
only if register width is 8 or 16 bits and also the memory place to
which regmap_read() should store it's result has non-zero upper bits,
which it seems doesn't happen anywhere in U-Boot normally. CI managed to
trigger this bug in unit test of dm_test_devm_regmap_field when compiled
for sandbox_defconfig using LTO.

Fix this by utilizing an union { u8; u16; u32; u64; } and reading data
into this union / writing data from this union.

Signed-off-by: Marek Behún <marek.be...@nic.cz>
Cc: Simon Glass <s...@chromium.org>
Cc: Heiko Schocher <h...@denx.de>
Cc: Bin Meng <bmeng...@gmail.com>
Cc: Pratyush Yadav <p.ya...@ti.com>
---
 drivers/core/regmap.c | 59 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 57 insertions(+), 2 deletions(-)

diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
index b51ce108c1..3206f3d112 100644
--- a/drivers/core/regmap.c
+++ b/drivers/core/regmap.c
@@ -435,7 +435,36 @@ int regmap_raw_read(struct regmap *map, uint offset, void 
*valp, size_t val_len)
 
 int regmap_read(struct regmap *map, uint offset, uint *valp)
 {
-       return regmap_raw_read(map, offset, valp, map->width);
+       union {
+               u8 v8;
+               u16 v16;
+               u32 v32;
+               u64 v64;
+       } u;
+       int res;
+
+       res = regmap_raw_read(map, offset, &u, map->width);
+       if (res)
+               return res;
+
+       switch (map->width) {
+       case REGMAP_SIZE_8:
+               *valp = u.v8;
+               break;
+       case REGMAP_SIZE_16:
+               *valp = u.v16;
+               break;
+       case REGMAP_SIZE_32:
+               *valp = u.v32;
+               break;
+       case REGMAP_SIZE_64:
+               *valp = u.v64;
+               break;
+       default:
+               unreachable();
+       }
+
+       return 0;
 }
 
 static inline void __write_8(u8 *addr, const u8 *val,
@@ -546,7 +575,33 @@ int regmap_raw_write(struct regmap *map, uint offset, 
const void *val,
 
 int regmap_write(struct regmap *map, uint offset, uint val)
 {
-       return regmap_raw_write(map, offset, &val, map->width);
+       union {
+               u8 v8;
+               u16 v16;
+               u32 v32;
+               u64 v64;
+       } u;
+
+       switch (map->width) {
+       case REGMAP_SIZE_8:
+               u.v8 = val;
+               break;
+       case REGMAP_SIZE_16:
+               u.v16 = val;
+               break;
+       case REGMAP_SIZE_32:
+               u.v32 = val;
+               break;
+       case REGMAP_SIZE_64:
+               u.v64 = val;
+               break;
+       default:
+               debug("%s: regmap size %zu unknown\n", __func__,
+                     (size_t)map->width);
+               return -EINVAL;
+       }
+
+       return regmap_raw_write(map, offset, &u, map->width);
 }
 
 int regmap_update_bits(struct regmap *map, uint offset, uint mask, uint val)
-- 
2.26.2

Reply via email to