[PATCH u-boot] regmap: fix a serious pointer casting bug

* [PATCH u-boot] regmap: fix a serious pointer casting bug
@ 2021-03-03 13:15 Marek Behún
  2021-03-05  4:09 ` Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Marek Behún @ 2021-03-03 13:15 UTC (permalink / raw)
  To: u-boot

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 simply by taking into account that regmap_raw_read() and
regmap_raw_write() behave as if the data given to these functions were
in little-endian format, i.e. use cpu_to_le32() / le32_to_cpu(). In
regmap_read() also zero out the space so that we don't get invalid
result if regmap_raw_read() does not fill the whole object.

Signed-off-by: Marek Beh?n <marek.behun@nic.cz>
---
 drivers/core/regmap.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
index b51ce108c1..5d37006fff 100644
--- a/drivers/core/regmap.c
+++ b/drivers/core/regmap.c
@@ -435,7 +435,16 @@ 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);
+	int res;
+
+	*valp = 0;
+	res = regmap_raw_read(map, offset, valp, map->width);
+	if (res)
+		return res;
+
+	*valp = le32_to_cpu(*valp);
+
+	return 0;
 }
 
 static inline void __write_8(u8 *addr, const u8 *val,
@@ -546,6 +555,8 @@ int regmap_raw_write(struct regmap *map, uint offset, const void *val,
 
 int regmap_write(struct regmap *map, uint offset, uint val)
 {
+	val = cpu_to_le32(val);
+
 	return regmap_raw_write(map, offset, &val, map->width);
 }
 
-- 
2.26.2

^ permalink raw reply related	[flat|nested] 6+ messages in thread