When dat_cond_set_storage_key() finds a large page, it will conditionally set the storage key in absolute memory using large_crste_to_phys() to get the absolute address. There is a race window between dat_entry_walk() and large_crste_to_phys(): the large page could have been split concurrently, and large_crste_to_phys() might be called with a crste that does not designate a large page, leading to crashes. Similar issues were also present in dat_set_storage_key(). dat_get_storage_key() and dat_reset_reference_bit() did instead check for a potential concurrent splitting of the large page, but then handled it incorrectly. Fix by performing a READ_ONCE on the crste pointer, checking and using the result, instead of dereferencing the pointer again. In case a race is detacted, try dat_entry_walk() again. Fixes: 8e03e8316eb2 ("KVM: s390: KVM page table management functions: storage keys") Signed-off-by: Claudio Imbrenda --- arch/s390/kvm/gmap/dat.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/arch/s390/kvm/gmap/dat.c b/arch/s390/kvm/gmap/dat.c index dcedd5479d82..ff80d02c9f56 100644 --- a/arch/s390/kvm/gmap/dat.c +++ b/arch/s390/kvm/gmap/dat.c @@ -621,17 +621,20 @@ int dat_get_storage_key(union asce asce, gfn_t gfn, union skey *skey) union pte *ptep; int rc; +again: skey->skey = 0; rc = dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep); if (rc) return rc; if (!ptep) { - union crste crste; + union crste crste = READ_ONCE(*crstep); - crste = READ_ONCE(*crstep); - if (!crste.h.fc || !crste.s.fc1.pr) + if (!crste_leaf(crste) && !crste.h.i) + goto again; + if (!crste.s.fc1.pr) return 0; + skey->skey = page_get_storage_key(large_crste_to_phys(crste, gfn)); return 0; } @@ -662,13 +665,20 @@ int dat_set_storage_key(struct kvm_s390_mmu_cache *mc, union asce asce, gfn_t gf union pte *ptep; int rc; +again: rc = dat_entry_walk(mc, gfn, asce, DAT_WALK_LEAF_ALLOC, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep); if (rc) return rc; if (!ptep) { - page_set_storage_key(large_crste_to_phys(*crstep, gfn), skey.skey, !nq); + union crste crste = READ_ONCE(*crstep); + + /* A large page has been split concurrently, try again */ + if (!crste_leaf(crste)) + goto again; + + page_set_storage_key(large_crste_to_phys(crste, gfn), skey.skey, !nq); return 0; } @@ -718,15 +728,22 @@ int dat_cond_set_storage_key(struct kvm_s390_mmu_cache *mmc, union asce asce, gf union pte *ptep; int rc; +again: rc = dat_entry_walk(mmc, gfn, asce, DAT_WALK_LEAF_ALLOC, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep); if (rc) return rc; if (!ptep) { + union crste crste = READ_ONCE(*crstep); + + /* A large page has been split concurrently, try again */ + if (!crste_leaf(crste)) + goto again; if (!oldkey) oldkey = &prev; - return page_cond_set_storage_key(large_crste_to_phys(*crstep, gfn), skey, oldkey, + + return page_cond_set_storage_key(large_crste_to_phys(crste, gfn), skey, oldkey, nq, mr, mc); } @@ -768,7 +785,7 @@ int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey) int rc; skey->skey = 0; - +again: rc = dat_entry_walk(NULL, gfn, asce, DAT_WALK_ANY, TABLE_TYPE_PAGE_TABLE, &crstep, &ptep); if (rc) return rc; @@ -776,9 +793,12 @@ int dat_reset_reference_bit(union asce asce, gfn_t gfn, union skey *skey) if (!ptep) { union crste crste = READ_ONCE(*crstep); - if (!crste.h.fc || !crste.s.fc1.pr) + /* A large page has been split concurrently, try again */ + if (!crste_leaf(crste) && !crste.h.i) + goto again; + if (!crste.s.fc1.pr) return 0; - skey->skey = page_reset_referenced(large_crste_to_phys(*crstep, gfn)) << 1; + skey->skey = page_reset_referenced(large_crste_to_phys(crste, gfn)) << 1; return 0; } old = pgste_get_lock(ptep); -- 2.55.0