| Message ID | 20240621092809.162-1-yuzenghui@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: Clear the initial ID map correctly before remapping | expand |
On Fri, 21 Jun 2024 at 11:28, Zenghui Yu <yuzenghui@huawei.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@huawei.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@kernel.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 >
On 2024/6/21 17:52, Ard Biesheuvel wrote: > On Fri, 21 Jun 2024 at 11:28, Zenghui Yu <yuzenghui@huawei.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@huawei.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. Yup! It took me a while to figure out why memset() with a VERY large size doesn't corrupt the memory. ;-) > 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. Agreed. I can fold it into the commit message if Will/Catalin ask for a respin. And it looks like an alternate "fix" would be just removing the memset(). > Nonetheless, > > Reviewed-by: Ard Biesheuvel <ardb@kernel.org> Thanks! Zenghui
On Fri, 21 Jun 2024 17:28:09 +0800, Zenghui Yu 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. > > > [...] Applied to arm64 (for-next/fixes), thanks! [1/1] arm64: Clear the initial ID map correctly before remapping https://git.kernel.org/arm64/c/ecc54006f158 Cheers,
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);
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@huawei.com> --- Found by code inspection (don't have the appropriate HW to test it). arch/arm64/kernel/pi/map_kernel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)