* [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
* [PATCH u-boot] regmap: fix a serious pointer casting bug
2021-03-03 13:15 [PATCH u-boot] regmap: fix a serious pointer casting bug Marek Behún
@ 2021-03-05 4:09 ` Simon Glass
2021-03-06 5:45 ` Heiko Schocher
2021-03-16 5:58 ` Simon Glass
2 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2021-03-05 4:09 UTC (permalink / raw
To: u-boot
On Wed, 3 Mar 2021 at 08:16, Marek Beh?n <marek.behun@nic.cz> wrote:
>
> 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(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH u-boot] regmap: fix a serious pointer casting bug
2021-03-03 13:15 [PATCH u-boot] regmap: fix a serious pointer casting bug Marek Behún
2021-03-05 4:09 ` Simon Glass
@ 2021-03-06 5:45 ` Heiko Schocher
2021-03-16 5:58 ` Simon Glass
2 siblings, 0 replies; 6+ messages in thread
From: Heiko Schocher @ 2021-03-06 5:45 UTC (permalink / raw
To: u-boot
Hello Marek,
On 03.03.21 14:15, Marek Beh?n wrote:
> 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(-)
Good catch, thanks!
Reviewed-by: Heiko Schocher <hs@denx.de>
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH u-boot] regmap: fix a serious pointer casting bug
2021-03-03 13:15 [PATCH u-boot] regmap: fix a serious pointer casting bug Marek Behún
2021-03-05 4:09 ` Simon Glass
2021-03-06 5:45 ` Heiko Schocher
@ 2021-03-16 5:58 ` Simon Glass
2021-03-16 8:46 ` Bin Meng
2 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2021-03-16 5:58 UTC (permalink / raw
To: u-boot
Hello Marek,
On 03.03.21 14:15, Marek Beh?n wrote:
> 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(-)
Good catch, thanks!
Reviewed-by: Heiko Schocher <hs@denx.de>
bye,
Heiko
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
Applied to u-boot-dm/next, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH u-boot] regmap: fix a serious pointer casting bug
2021-03-16 5:58 ` Simon Glass
@ 2021-03-16 8:46 ` Bin Meng
2021-03-17 3:46 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2021-03-16 8:46 UTC (permalink / raw
To: u-boot
Hi Simon,
On Tue, Mar 16, 2021 at 2:10 PM Simon Glass <sjg@chromium.org> wrote:
>
> Hello Marek,
>
> On 03.03.21 14:15, Marek Beh?n wrote:
> > 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(-)
>
> Good catch, thanks!
>
> Reviewed-by: Heiko Schocher <hs@denx.de>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
>
> Applied to u-boot-dm/next, thanks!
This patch is included in the LTO series.
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH u-boot] regmap: fix a serious pointer casting bug
2021-03-16 8:46 ` Bin Meng
@ 2021-03-17 3:46 ` Simon Glass
0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2021-03-17 3:46 UTC (permalink / raw
To: u-boot
Hi Bin,
On Tue, 16 Mar 2021 at 21:46, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Mar 16, 2021 at 2:10 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hello Marek,
> >
> > On 03.03.21 14:15, Marek Beh?n wrote:
> > > 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(-)
> >
> > Good catch, thanks!
> >
> > Reviewed-by: Heiko Schocher <hs@denx.de>
> >
> > bye,
> > Heiko
> > --
> > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
> > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> > Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: hs at denx.de
> >
> > Applied to u-boot-dm/next, thanks!
>
> This patch is included in the LTO series.
OK, I'll drop it.
Regards,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-03-17 3:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-03 13:15 [PATCH u-boot] regmap: fix a serious pointer casting bug Marek Behún
2021-03-05 4:09 ` Simon Glass
2021-03-06 5:45 ` Heiko Schocher
2021-03-16 5:58 ` Simon Glass
2021-03-16 8:46 ` Bin Meng
2021-03-17 3:46 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.