[RFC PATCH 1/3] KVM: arm64: Fix possible memory leak in kvm stage2
wangyanan55 at huawei.com
Tue Dec 1 02:21:23 EST 2020
On 2020/11/30 21:21, Will Deacon wrote:
> On Mon, Nov 30, 2020 at 08:18:45PM +0800, Yanan Wang wrote:
>> When installing a new leaf pte onto an invalid ptep, we need to get_page(ptep).
>> When just updating a valid leaf ptep, we shouldn't get_page(ptep).
>> Incorrect page_count of translation tables might lead to memory leak,
>> when unmapping a stage 2 memory range.
> Did you find this by inspection, or did you hit this in practice? I'd be
> interested to see the backtrace for mapping over an existing mapping.
Actually this is found by inspection.
In the current code, get_page() will uniformly called at "out_get_page"
in function stage2_map_walk_leaf(),
no matter the old ptep is valid or not.
When using stage2_unmap_walker() API to unmap a memory range, some
page-table pages might not be
freed if page_count of the pages is not right.
>> Signed-off-by: Yanan Wang <wangyanan55 at huawei.com>
>> arch/arm64/kvm/hyp/pgtable.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>> diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c
>> index 0271b4a3b9fe..696b6aa83faf 100644
>> --- a/arch/arm64/kvm/hyp/pgtable.c
>> +++ b/arch/arm64/kvm/hyp/pgtable.c
>> @@ -186,6 +186,7 @@ static bool kvm_set_valid_leaf_pte(kvm_pte_t *ptep, u64 pa, kvm_pte_t attr,
>> return old == pte;
>> smp_store_release(ptep, pte);
>> + get_page(virt_to_page(ptep));
> This is also used for the hypervisor stage-1 page-table, so I'd prefer to
> leave this function as-is.
I agree at this point.
>> return true;
>> @@ -476,6 +477,7 @@ static bool stage2_map_walker_try_leaf(u64 addr, u64 end, u32 level,
>> /* There's an existing valid leaf entry, so perform break-before-make */
>> kvm_call_hyp(__kvm_tlb_flush_vmid_ipa, data->mmu, addr, level);
>> + put_page(virt_to_page(ptep));
>> kvm_set_valid_leaf_pte(ptep, phys, data->attr, level);
>> data->phys += granule;
> Isn't this hunk alone sufficient to solve the problem?
Not sufficient enough. When the old ptep is valid and old pte equlas new
pte, in this case, "True" is also returned by kvm_set_valid_leaf_pte()
and get_page() will still be called.
More information about the linux-arm-kernel