From: Marc Zyngier <maz@kernel.org> Date: 2021-08-02 12:41:22
This is a rework of the patch previously posted at [1].
The gist of the problem is that kmemleak can legitimately access data
that has been removed from the kernel view, for two reasons:
(1) .hyp.rodata is lumped together with the BSS
(2) there is no separation of the HYP BSS from the kernel BSS
(1) can easily be addressed by moving the .hyp.rodata section into the
kernel's RO zone, which avoids any kmemleak into that section.
(2) must be addressed by telling kmemleak about the address range.
Tested on a SC2A11 system, in protected and non-protected modes with
kmemleak active. Both patches are stable candidates.
[1] https://lore.kernel.org/r/20210729135016.3037277-1-maz@kernel.org
Marc Zyngier (2):
arm64: Move .hyp.rodata outside of the _sdata.._edata range
KVM: arm64: Unregister HYP sections from kmemleak in protected mode
arch/arm64/kernel/vmlinux.lds.S | 4 ++--
arch/arm64/kvm/arm.c | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-08-02 12:38:56
The HYP rodata section is currently lumped together with the BSS,
which isn't exactly what is expected (it gets registered with
kmemleak, for example).
Move it away so that it is actually marked RO. As an added
benefit, it isn't registered with kmemleak anymore.
Fixes: 380e18ade4a5 ("KVM: arm64: Introduce a BSS section for use at Hyp")
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org #5.13
---
arch/arm64/kernel/vmlinux.lds.S | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
From: Marc Zyngier <maz@kernel.org> Date: 2021-08-02 12:38:56
Booting a KVM host in protected mode with kmemleak quickly results
in a pretty bad crash, as kmemleak doesn't know that the HYP sections
have been taken away. This is specially true for the BSS section,
which is part of the kernel BSS section and registered at boot time
by kmemleak itself.
Unregister the HYP part of the BSS before making that section
HYP-private. The rest of the HYP-specific data is obtained via
the page allocator or lives in other sections, none of which is
subjected to kmemleak.
Fixes: 90134ac9cabb ("KVM: arm64: Protect the .hyp sections from the host")
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: stable@vger.kernel.org # 5.13
---
arch/arm64/kvm/arm.c | 7 +++++++
1 file changed, 7 insertions(+)
Hi Marc,
On Monday 02 Aug 2021 at 13:38:29 (+0100), Marc Zyngier wrote:
The HYP rodata section is currently lumped together with the BSS,
which isn't exactly what is expected (it gets registered with
kmemleak, for example).
Move it away so that it is actually marked RO. As an added
benefit, it isn't registered with kmemleak anymore.
2d7bf218ca73 ("KVM: arm64: Add .hyp.data..ro_after_init ELF section")
states explicitly that the hyp ro_after_init section should remain RW in
the host as it is expected to modify it before initializing EL2. But I
can't seem to trigger anything with this patch applied, so I'll look
into this a bit more.
Fixes: 380e18ade4a5 ("KVM: arm64: Introduce a BSS section for use at Hyp")
From: Marc Zyngier <maz@kernel.org> Date: 2021-08-02 13:20:46
Hi Quentin,
On Mon, 02 Aug 2021 14:11:07 +0100,
Quentin Perret [off-list ref] wrote:
Hi Marc,
On Monday 02 Aug 2021 at 13:38:29 (+0100), Marc Zyngier wrote:
quoted
The HYP rodata section is currently lumped together with the BSS,
which isn't exactly what is expected (it gets registered with
kmemleak, for example).
Move it away so that it is actually marked RO. As an added
benefit, it isn't registered with kmemleak anymore.
2d7bf218ca73 ("KVM: arm64: Add .hyp.data..ro_after_init ELF section")
states explicitly that the hyp ro_after_init section should remain RW in
the host as it is expected to modify it before initializing EL2. But I
can't seem to trigger anything with this patch applied, so I'll look
into this a bit more.
The switch to RO happens quite late. And if the host was to actually
try and change things there, it would be screwed anyway (we will have
already removed the pages from its S2).
I wouldn't be surprised if this was a consequence of the way we now
build the HYP object, and the comment in the original commit may not
be valid anymore.
quoted
Fixes: 380e18ade4a5 ("KVM: arm64: Introduce a BSS section for use at Hyp")
Not sure this is the patch to blame?
My bad, this is plain wrong. I'm not sure it can be applied earlier
though if my rambling above is correct.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Monday 02 Aug 2021 at 14:20:42 (+0100), Marc Zyngier wrote:
Hi Quentin,
On Mon, 02 Aug 2021 14:11:07 +0100,
Quentin Perret [off-list ref] wrote:
quoted
Hi Marc,
On Monday 02 Aug 2021 at 13:38:29 (+0100), Marc Zyngier wrote:
quoted
The HYP rodata section is currently lumped together with the BSS,
which isn't exactly what is expected (it gets registered with
kmemleak, for example).
Move it away so that it is actually marked RO. As an added
benefit, it isn't registered with kmemleak anymore.
2d7bf218ca73 ("KVM: arm64: Add .hyp.data..ro_after_init ELF section")
states explicitly that the hyp ro_after_init section should remain RW in
the host as it is expected to modify it before initializing EL2. But I
can't seem to trigger anything with this patch applied, so I'll look
into this a bit more.
The switch to RO happens quite late. And if the host was to actually
try and change things there, it would be screwed anyway (we will have
already removed the pages from its S2).
Yes, clearly mapping rodata RO in host happens much later than I
thought, so this should indeed be fine.
I wouldn't be surprised if this was a consequence of the way we now
build the HYP object, and the comment in the original commit may not
be valid anymore.
Just had a quick look and that still seems valid, at least for some
things (e.g. see how we set hyp_cpu_logical_map[] early from EL1 while
it is clearly annotated as __ro_after_init in the EL2 code).
quoted
quoted
Fixes: 380e18ade4a5 ("KVM: arm64: Introduce a BSS section for use at Hyp")
Not sure this is the patch to blame?
My bad, this is plain wrong. I'm not sure it can be applied earlier
though if my rambling above is correct.
By the look of it going all the way back to 2d7bf218ca73 (in David's
PSCI proxy series) should actually be correct. But not sure if that's
really going to make a difference before the patch you've mentioned
above as the kmemleak issue will only be visible once we have a host
stage-2, so no big deal.
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Aug 02, 2021 at 01:38:30PM +0100, Marc Zyngier wrote:
Booting a KVM host in protected mode with kmemleak quickly results
in a pretty bad crash, as kmemleak doesn't know that the HYP sections
have been taken away. This is specially true for the BSS section,
which is part of the kernel BSS section and registered at boot time
by kmemleak itself.
Unregister the HYP part of the BSS before making that section
HYP-private. The rest of the HYP-specific data is obtained via
the page allocator or lives in other sections, none of which is
subjected to kmemleak.
Fixes: 90134ac9cabb ("KVM: arm64: Protect the .hyp sections from the host")
Reviewed-by: Quentin Perret <redacted>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: stable@vger.kernel.org # 5.13
On Mon, Aug 02, 2021 at 01:38:29PM +0100, Marc Zyngier wrote:
The HYP rodata section is currently lumped together with the BSS,
which isn't exactly what is expected (it gets registered with
kmemleak, for example).
Move it away so that it is actually marked RO. As an added
benefit, it isn't registered with kmemleak anymore.
Fixes: 380e18ade4a5 ("KVM: arm64: Introduce a BSS section for use at Hyp")
Suggested-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org #5.13
From: Marc Zyngier <maz@kernel.org> Date: 2021-08-04 12:28:55
On Mon, 2 Aug 2021 13:38:28 +0100, Marc Zyngier wrote:
This is a rework of the patch previously posted at [1].
The gist of the problem is that kmemleak can legitimately access data
that has been removed from the kernel view, for two reasons:
(1) .hyp.rodata is lumped together with the BSS
(2) there is no separation of the HYP BSS from the kernel BSS
[...]
Applied to next, thanks!
[1/2] arm64: Move .hyp.rodata outside of the _sdata.._edata range
commit: eb48d154cd0dade56a0e244f0cfa198ea2925ed3
[2/2] KVM: arm64: Unregister HYP sections from kmemleak in protected mode
commit: 47e6223c841e029bfc23c3ce594dac5525cebaf8
Cheers,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel