[PATCH u-boot v3.1 01/39] regmap: fix a serious pointer casting bug

[Marek Behun]

On Tue, 16 Mar 2021 22:04:17 +0530
Pratyush Yadav <p.yadav at ti.com> wrote:

> > +	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;  
> 
> I think this should fix the problem you are trying to solve.
> 
> But I see another problem with this code. What if someone wants to read 
> 8 bytes? IIUC, since valp points to a uint, *valp = u.v64 will result in 
> 4 bytes being truncated from the result.
> 
> I see two options:
> 
> - Change the uint pointer to u64 pointer and update every driver to use 
>   a u64 when using the regmap.
> 
> - Change the uint pointer to void pointer and expect every driver to 
>   pass a container with exactly the required size, based on map->width. 
>   Update the ones that don't follow this.
> 
> I prefer the latter option.

If only these two options are possible, then the first option is
better. The regmap_read function ought to be simple, so no void *
pointer.

There is another option: if a value greater than ULONG_MAX is read, the
regmap_read() function will return -ERANGE. This way this function
will remain simple. ulong is 64-bit wide on 64-bit devices, so this
only is a problem when a 64-bit wide regmap is used on a 32-bit device.
I think we should keep regmap_read() simple, and for people who use a
64-bit wide regmap on 32-bit device, there is regmap_raw_read().

But I think this problem should be solved (however we decide) in a
follow-up patch. This patch fixes the pointer casting bug, and commits
should remain atomic (fixing one thing).

Marek