linux-kernel - Re: [PATCH] regmap: don't check for alignment when using reg

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZEKwxhJJNkuX7VTr@colin-ia-desktop>
Date:   Fri, 21 Apr 2023 08:50:30 -0700
From:   Colin Foster <colin.foster@...advantage.com>
To:     Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc:     Mark Brown <broonie@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        Lee Jones <lee@...nel.org>, linux-kernel@...r.kernel.org,
        thomas.petazzoni@...tlin.com, alexis.lothore@...tlin.com
Subject: Re: [PATCH] regmap: don't check for alignment when using reg_shift

Hi Maxime,

On Thu, Apr 20, 2023 at 05:06:17PM +0200, Maxime Chevallier wrote:
> On regmap consumers that require address translation through
> up/downshifting, the alignment check in the regmap core doesn't take the
> translation into account. This doesn't matter when downshifting the
> register address, as any address that fits a given alignment requirement
> will still meet it when downshifted (a 4-byte aligned address will
> always also be 2-bytes aligned for example).
> 
> However, when upshifting, this check causes spurious errors, as it
> occurs before the upshifting.

I don't follow why upshifting should make a difference to alignment.
Assuming it does though, would it make sense to test

map->format.reg_shift > 0

instead of just !map->format.reg_shift?

> -	if (!IS_ALIGNED(reg, map->reg_stride))
> +	if (!map->format.reg_shift && !IS_ALIGNED(reg, map->reg_stride))
>  		return -EINVAL;

In the case of ocelot_spi, we'd want to flag an invalid access to a
register like 0x71070003... Before this patch it would return -EINVAL,
after this patch it would access 0x71070000.

Colin Foster