On 16/03/21 03:15PM, Marek Behun wrote:
> On Tue, 16 Mar 2021 19:28:46 +0530
> Pratyush Yadav <p.ya...@ti.com> wrote:
>
> > On 16/03/21 01:25PM, 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.be...@nic.cz>
> > > Reviewed-by: Simon Glass <s...@chromium.org>
> > > Reviewed-by: Heiko Schocher <h...@denx.de>
> > > Reviewed-by: Bin Meng <bmeng...@gmail.com>
> > > ---
> > > 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);
> >
> > Looks like I'm a bit late to the party and Simon has already applied
> > this patch.
>
> Where did he apply? I don't see it applied in u-boot-dm.
Message-ID:
<CAPnjgZ3rE-UGjBk+sHasZy=afbprg3yxyg8oyaihx6yrug3...@mail.gmail.com>
The quoting is off but look at the last line, it says "applied to
u-boot-dm/next".
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
