Skip to content

Commit c148dc8

Browse files
author
Claudio Imbrenda
committed
KVM: s390: fix race in gmap_make_secure()
Fix a potential race in gmap_make_secure() and remove the last user of follow_page() without FOLL_GET. The old code is locking something it doesn't have a reference to, and as explained by Jason and David in this discussion: https://lore.kernel.org/linux-mm/Y9J4P%2FRNvY1Ztn0Q@nvidia.com/ it can lead to all kind of bad things, including the page getting unmapped (MADV_DONTNEED), freed, reallocated as a larger folio and the unlock_page() would target the wrong bit. There is also another race with the FOLL_WRITE, which could race between the follow_page() and the get_locked_pte(). The main point is to remove the last use of follow_page() without FOLL_GET or FOLL_PIN, removing the races can be considered a nice bonus. Link: https://lore.kernel.org/linux-mm/Y9J4P%2FRNvY1Ztn0Q@nvidia.com/ Suggested-by: Jason Gunthorpe <jgg@nvidia.com> Fixes: 214d9bb ("s390/mm: provide memory management functions for protected KVM guests") Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Message-Id: <20230428092753.27913-2-imbrenda@linux.ibm.com>
1 parent 292a7d6 commit c148dc8

1 file changed

Lines changed: 11 additions & 21 deletions

File tree

  • arch/s390/kernel

arch/s390/kernel/uv.c

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -192,21 +192,10 @@ static int expected_page_refs(struct page *page)
192192
return res;
193193
}
194194

195-
static int make_secure_pte(pte_t *ptep, unsigned long addr,
196-
struct page *exp_page, struct uv_cb_header *uvcb)
195+
static int make_page_secure(struct page *page, struct uv_cb_header *uvcb)
197196
{
198-
pte_t entry = READ_ONCE(*ptep);
199-
struct page *page;
200197
int expected, cc = 0;
201198

202-
if (!pte_present(entry))
203-
return -ENXIO;
204-
if (pte_val(entry) & _PAGE_INVALID)
205-
return -ENXIO;
206-
207-
page = pte_page(entry);
208-
if (page != exp_page)
209-
return -ENXIO;
210199
if (PageWriteback(page))
211200
return -EAGAIN;
212201
expected = expected_page_refs(page);
@@ -304,17 +293,18 @@ int gmap_make_secure(struct gmap *gmap, unsigned long gaddr, void *uvcb)
304293
goto out;
305294

306295
rc = -ENXIO;
307-
page = follow_page(vma, uaddr, FOLL_WRITE);
308-
if (IS_ERR_OR_NULL(page))
309-
goto out;
310-
311-
lock_page(page);
312296
ptep = get_locked_pte(gmap->mm, uaddr, &ptelock);
313-
if (should_export_before_import(uvcb, gmap->mm))
314-
uv_convert_from_secure(page_to_phys(page));
315-
rc = make_secure_pte(ptep, uaddr, page, uvcb);
297+
if (pte_present(*ptep) && !(pte_val(*ptep) & _PAGE_INVALID) && pte_write(*ptep)) {
298+
page = pte_page(*ptep);
299+
rc = -EAGAIN;
300+
if (trylock_page(page)) {
301+
if (should_export_before_import(uvcb, gmap->mm))
302+
uv_convert_from_secure(page_to_phys(page));
303+
rc = make_page_secure(page, uvcb);
304+
unlock_page(page);
305+
}
306+
}
316307
pte_unmap_unlock(ptep, ptelock);
317-
unlock_page(page);
318308
out:
319309
mmap_read_unlock(gmap->mm);
320310

0 commit comments

Comments
 (0)