linux-kernel - Re: [PATCH] arm64: Clear the initial ID map correctly before remapping

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAMj1kXEH0ohn57DUrCu6S-AJW=B9CyrpMwyabpjBpD9tD4VV=A@mail.gmail.com>
Date: Fri, 21 Jun 2024 11:52:47 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Zenghui Yu <yuzenghui@...wei.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	catalin.marinas@....com, will@...nel.org, wanghaibin.wang@...wei.com
Subject: Re: [PATCH] arm64: Clear the initial ID map correctly before remapping

On Fri, 21 Jun 2024 at 11:28, Zenghui Yu <yuzenghui@...wei.com> wrote:
>
> In the attempt to clear and recreate the initial ID map for LPA2, we
> wrongly use 'start - end' as the map size and make the memset() almost a
> nop.
>
> Fix it by passing the correct map size.
>
> Fixes: 9684ec186f8f ("arm64: Enable LPA2 at boot if supported by the system")
> Signed-off-by: Zenghui Yu <yuzenghui@...wei.com>
> ---
>
> Found by code inspection (don't have the appropriate HW to test it).
>

Good catch!

Even though memset() takes an unsigned size_t, the zeroing path in
arm64's memset.S does a signed compare on the provided size, and will
zero at most 63 bytes if the size has the sign bit set. So in the end,
it does not clear anything. Note that in this particular case, that
doesn't actually matter - the memory is reused immediately to create
another copy of the ID map, and any unused regions containing garbage
will just be ignored.

Nonetheless,

Reviewed-by: Ard Biesheuvel <ardb@...nel.org>




>  arch/arm64/kernel/pi/map_kernel.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c
> index 5fa08e13e17e..f374a3e5a5fe 100644
> --- a/arch/arm64/kernel/pi/map_kernel.c
> +++ b/arch/arm64/kernel/pi/map_kernel.c
> @@ -173,7 +173,7 @@ static void __init remap_idmap_for_lpa2(void)
>          * Don't bother with the FDT, we no longer need it after this.
>          */
>         memset(init_idmap_pg_dir, 0,
> -              (u64)init_idmap_pg_dir - (u64)init_idmap_pg_end);
> +              (u64)init_idmap_pg_end - (u64)init_idmap_pg_dir);
>
>         create_init_idmap(init_idmap_pg_dir, mask);
>         dsb(ishst);
> --
> 2.33.0
>