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
| ||
|
Date: Thu, 14 Apr 2022 13:30:33 +0200 From: Janosch Frank <frankja@...ux.ibm.com> To: Claudio Imbrenda <imbrenda@...ux.ibm.com>, kvm@...r.kernel.org Cc: borntraeger@...ibm.com, thuth@...hat.com, pasic@...ux.ibm.com, david@...hat.com, linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org, scgl@...ux.ibm.com, mimu@...ux.ibm.com, nrb@...ux.ibm.com Subject: Re: [PATCH v10 01/19] KVM: s390: pv: leak the topmost page table when destroy fails On 4/14/22 10:02, Claudio Imbrenda wrote: > Each secure guest must have a unique ASCE (address space control > element); we must avoid that new guests use the same page for their > ASCE, to avoid errors. > > Since the ASCE mostly consists of the address of the topmost page table > (plus some flags), we must not return that memory to the pool unless > the ASCE is no longer in use. > > Only a successful Destroy Secure Configuration UVC will make the ASCE > reusable again. > > If the Destroy Configuration UVC fails, the ASCE cannot be reused for a > secure guest (either for the ASCE or for other memory areas). To avoid > a collision, it must not be used again. This is a permanent error and > the page becomes in practice unusable, so we set it aside and leak it. > On failure we already leak other memory that belongs to the ultravisor > (i.e. the variable and base storage for a guest) and not leaking the > topmost page table was an oversight. > > This error (and thus the leakage) should not happen unless the hardware > is broken or KVM has some unknown serious bug. > Reviewed-by: Janosch Frank <frankja@...ux.ibm.com> > + /* > + * In case the ASCE needs to be "removed" multiple times, for example > + * if the VM is rebooted into secure mode several times > + * concurrently, or if s390_replace_asce fails after calling > + * s390_remove_old_asce and is attempted again later. In that case > + * the old asce has been removed from the list, and therefore it > + * will not be freed when the VM terminates, but the ASCE is still > + * in use and still pointed to. > + * A subsequent call to replace_asce will follow the pointer and try > + * to remove the same page from the list again. > + * Therefore it's necessary that the page of the ASCE has valid > + * pointers, so list_del can work (and do nothing) without > + * dereferencing stale or invalid pointers. > + */ > + INIT_LIST_HEAD(&old->lru); > + spin_unlock(&gmap->guest_table_lock); > +} > +EXPORT_SYMBOL_GPL(s390_remove_old_asce); > + > +/** > + * s390_replace_asce - Try to replace the current ASCE of a gmap with > + * another equivalent one. with a copy? > + * @gmap the gmap > + * > + * If the allocation of the new top level page table fails, the ASCE is not > + * replaced. > + * In any case, the old ASCE is always removed from the list. Therefore the removed from the gmap crst list > + * caller has to make sure to save a pointer to it beforehands, unless an > + * intentional leak is intended. > + */ > +int s390_replace_asce(struct gmap *gmap) > +{ > + unsigned long asce; > + struct page *page; > + void *table; > + > + s390_remove_old_asce(gmap); > + > + page = alloc_pages(GFP_KERNEL_ACCOUNT, CRST_ALLOC_ORDER); > + if (!page) > + return -ENOMEM; > + table = page_to_virt(page); > + memcpy(table, gmap->table, 1UL << (CRST_ALLOC_ORDER + PAGE_SHIFT)); > + > + /* > + * The caller has to deal with the old ASCE, but here we make sure > + * the new one is properly added to the list of page tables, so that > + * it will be freed when the VM is torn down. > + */ > + spin_lock(&gmap->guest_table_lock); > + list_add(&page->lru, &gmap->crst_list); > + spin_unlock(&gmap->guest_table_lock); > + > + /* Set new table origin while preserving existing ASCE control bits */ > + asce = (gmap->asce & ~_ASCE_ORIGIN) | __pa(table); > + WRITE_ONCE(gmap->asce, asce); > + WRITE_ONCE(gmap->mm->context.gmap_asce, asce); > + WRITE_ONCE(gmap->table, table); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(s390_replace_asce);
Powered by blists - more mailing lists