Resend: correctness race when paging-in
PATCH 1/1 Resend: correctness race when paging-in
P2m_mem_paging_prep ensures that an mfn is backing the paged-out gfn, and transitions to the next state in the paging state machine for this page. Foreign mappings of the gfn will now succeed. This is the key idea, as it allows the pager to now map the gfn and fill in its contents.
Unfortunately, it also allows any other foreign mapper to map the gfn and read its contents. This is particularly dangerous when the populate is launched by a foreign mapper in the first place, which will be actively retrying the map operation and might race with the pager. Qemu-dm being a prime example.
Fix the race by allowing a buffer to be optionally passed in the prep operation, and having the hypervisor memcpy from that buffer into the newly prepped page before promoting the gfn type.
Second patch is a tools patch.
Resent after feedback: xenpaging patch attached, simplified with use of copy_from_guest. Left potntial short-cut to avoid pging_resume for further discussion.
xen/arch/x86/mm/mem_event.c | 2 +- xen/arch/x86/mm/mem_paging.c | 2 +- xen/arch/x86/mm/p2m.c | 32 ++++++++++++++++++++++++++++++-- xen/include/asm-x86/p2m.h | 2 +- xen/include/public/domctl.h | 8 ++++++-- tools/libxc/xc_mem_event.c | 4 ++-- tools/libxc/xc_mem_paging.c | 23 +++++++++++++++++++++++ tools/libxc/xenctrl.h | 2 ++ tools/xenpaging/xenpaging.c | 43 +++++++++++++++++++++---------------------- 9 files changed, 87 insertions(+), 31 deletions(-)
PATCH 1/1 After preparing a page for page-in, allow immediate fill-in of the page contents
xen/arch/x86/mm/mem_event.c | 2 +- xen/arch/x86/mm/mem_paging.c | 2 +- xen/arch/x86/mm/p2m.c | 32 ++++++++++++++++++++++++++++++-- xen/include/asm-x86/p2m.h | 2 +- xen/include/public/domctl.h | 8 ++++++-- 5 files changed, 39 insertions(+), 7 deletions(-)
p2m_mem_paging_prep ensures that an mfn is backing the paged-out gfn, and transitions to the next state in the paging state machine for that page. Foreign mappings of the gfn will now succeed. This is the key idea, as it allows the pager to now map the gfn and fill in its contents.
Unfortunately, it also allows any other foreign mapper to map the gfn and read its contents. This is particularly dangerous when the populate is launched by a foreign mapper in the first place, which will be actively retrying the map operation and might race with the pager. Qemu-dm being a prime example.
Fix the race by allowing a buffer to be optionally passed in the prep operation, and having the hypervisor memcpy from that buffer into the newly prepped page before promoting the gfn type.
xen/arch/x86/mm/mem_event.c +1 -1
@@ -45,7 +45,7 @@ static int mem_event_enable(struct domaistruct domain *dom_mem_event = current->domain;struct vcpu *v = current;unsigned long ring_addr = mec->ring_addr; -unsigned long shared_addr = mec->shared_addr; +unsigned long shared_addr = mec->u.shared_addr;l1_pgentry_t l1e;unsigned long shared_gfn =0 , ring_gfn =0 ;/* gcc ... */ p2m_type_t p2mt;
xen/arch/x86/mm/mem_paging.c +1 -1
@@ -47,7 +47,7 @@ int mem_paging_domctl(struct domain *d,case XEN_DOMCTL_MEM_EVENT_OP_PAGING_PREP : {unsigned long gfn = mec->gfn; -return p2m_mem_paging_prep (d, gfn); +return p2m_mem_paging_prep (d, gfn, mec->u.buffer); }break ;
xen/arch/x86/mm/p2m.c +30 -2
@@ -983,14 +983,21 @@ void p2m_mem_paging_populate(struct doma* mfn if populate was called for gfn which was nominated but not evicted. In * this case only the p2mt needs to be forwarded. */ -int p2m_mem_paging_prep (struct domain *d,unsigned long gfn) +int p2m_mem_paging_prep (struct domain *d,unsigned long gfn,uint64_t buffer) {struct page_info *page;p2m_type_t p2mt;p2m_access_t a;mfn_t mfn;struct p2m_domain *p2m =p2m_get_hostp2m (d); -int ret; +int ret, page_extant =1 ; +const void *user_ptr = (const void *) buffer; + +if ( user_ptr ) +/* Sanity check the buffer and bail out early if trouble */ +if ( (buffer & (PAGE_SIZE -1 )) || + (!access_ok (user_ptr,PAGE_SIZE )) ) +return -EINVAL ;p2m_lock (p2m);
@@ -1010,6 +1017,27 @@ int p2m_mem_paging_prep(struct domain *dif (unlikely (page ==NULL ) ) goto out; mfn =page_to_mfn (page); + page_extant =0 ; + } + +/* If we were given a buffer, now is the time to use it */ +if ( !page_extant && user_ptr ) + { +void *guest_map; +int rc; + +ASSERT (mfn_valid (mfn) ); + guest_map =map_domain_page (mfn_x (mfn)); + rc =copy_from_user (guest_map, user_ptr,PAGE_SIZE ); +unmap_domain_page (guest_map); +if ( rc ) + { +gdprintk (XENLOG_ERR ,"Failed to load paging-in gfn %lx domain %u " +"bytes left %d\n" , gfn, d->domain_id, rc); + ret = -EFAULT ; +put_page (page);/* Don't leak pages */ + goto out; + } }/* Fix p2m mapping */
xen/include/asm-x86/p2m.h +1 -1
@@ -477,7 +477,7 @@ void p2m_mem_paging_drop_page(struct dom/* Start populating a paged out frame */ void p2m_mem_paging_populate (struct domain *d,unsigned long gfn);/* Prepare the p2m for paging a frame in */ -int p2m_mem_paging_prep (struct domain *d,unsigned long gfn); +int p2m_mem_paging_prep (struct domain *d,unsigned long gfn,uint64_t buffer);/* Resume normal operation (in case a domain was paused) */ void p2m_mem_paging_resume (struct domain *d);#else
xen/include/public/domctl.h +6 -2
@@ -742,8 +742,12 @@ struct xen_domctl_mem_event_op {uint32_t op;/* XEN_DOMCTL_MEM_EVENT_OP_*_* */ uint32_t mode;/* XEN_DOMCTL_MEM_EVENT_OP_* */ -/* OP_ENABLE */ -uint64_aligned_t shared_addr;/* IN: Virtual address of shared page */ +union { +/* OP_ENABLE IN: Virtual address of shared page */ +uint64_aligned_t shared_addr; +/* PAGING_PREP IN: buffer to immediately fill page in */ +uint64_aligned_t buffer; + } u;uint64_aligned_t ring_addr;/* IN: Virtual address of ring page */ /* Other OPs */
PATCH 1/1 Tools: Libxc wrappers to automatically fill in page oud page contents on prepare
In reply to Andres Lagar-Cavillatools/libxc/xc_mem_event.c | 4 ++-- tools/libxc/xc_mem_paging.c | 23 +++++++++++++++++++++++ tools/libxc/xenctrl.h | 2 ++ 3 files changed, 27 insertions(+), 2 deletions(-)
tools/libxc/xc_mem_event.c +2 -2
@@ -24,7 +24,7 @@#include "xc_private.h" int xc_mem_event_control (xc_interface *xch,domid_t domain_id,unsigned int op, -unsigned int mode,void *shared_page, +unsigned int mode,void *page,void *ring_page,unsigned long gfn) {DECLARE_DOMCTL ;
@@ -34,7 +34,7 @@ int xc_mem_event_control(xc_interface *x domctl.u.mem_event_op.op = op; domctl.u.mem_event_op.mode = mode; - domctl.u.mem_event_op.shared_addr = (unsigned long )shared_page; + domctl.u.mem_event_op.u.shared_addr = (unsigned long )page; domctl.u.mem_event_op.ring_addr = (unsigned long )ring_page; domctl.u.mem_event_op.gfn = gfn;
tools/libxc/xc_mem_paging.c +23 -0
@@ -65,6 +65,29 @@ int xc_mem_paging_prep(xc_interface *xch NULL, NULL, gfn); } +int xc_mem_paging_load (xc_interface *xch,domid_t domain_id, +unsigned long gfn,void *buffer) +{ +int rc; + +if ( !buffer ) +return -EINVAL ; + +if ( ((unsigned long ) buffer) & (XC_PAGE_SIZE -1 ) ) +return -EINVAL ; + +if (mlock (buffer,XC_PAGE_SIZE ) ) +return -errno; + + rc =xc_mem_event_control (xch, domain_id, +XEN_DOMCTL_MEM_EVENT_OP_PAGING_PREP , +XEN_DOMCTL_MEM_EVENT_OP_PAGING , + buffer,NULL , gfn); + + (void )munlock (buffer,XC_PAGE_SIZE ); +return rc; +} +int xc_mem_paging_resume (xc_interface *xch,domid_t domain_id,unsigned long gfn) { return xc_mem_event_control(xch, domain_id,
tools/libxc/xenctrl.h +2 -0
@@ -1839,6 +1839,8 @@ int xc_mem_paging_nominate(xc_interface unsigned long gfn);int xc_mem_paging_evict (xc_interface *xch,domid_t domain_id,unsigned long gfn);int xc_mem_paging_prep (xc_interface *xch,domid_t domain_id,unsigned long gfn); +int xc_mem_paging_load (xc_interface *xch,domid_t domain_id, +unsigned long gfn,void *buffer);int xc_mem_paging_resume (xc_interface *xch,domid_t domain_id,unsigned long gfn);
PATCH 1/1 Teach xenpaging to use the new and non-racy xc_mem_paging_load
In reply to Andres Lagar-Cavillatools/xenpaging/xenpaging.c | 43 +++++++++++++++++++++---------------------- 1 files changed, 21 insertions(+), 22 deletions(-)
interface.
tools/xenpaging/xenpaging.c +21 -22
@@ -45,6 +45,7 @@ static char *dom_path;static char watch_token[16 ];static char *filename;static int interrupted; +static void *paging_buffer =NULL ;static void unlink_pagefile (void ) {
@@ -438,6 +439,13 @@ static xenpaging_t *xenpaging_init(int a goto err; } + paging_buffer =init_page (); +if ( !paging_buffer ) + { +ERROR ("Creating page aligned load buffer" ); + goto err; + } +return paging; err:
@@ -649,10 +657,20 @@ static int xenpaging_populate_page(xenpaunsigned char oom =0 ;DPRINTF ("populate_page < gfn %" PRI_xen_pfn" pageslot %d\n" , gfn, i); + +/* Read page */ + ret =read_page (fd, paging_buffer, i); +if ( ret !=0 ) + { +ERROR ("Error reading page" ); + goto out; + } + do { /* Tell Xen to allocate a page for the domain */ - ret = xc_mem_paging_prep(xch, paging->mem_event.domain_id, gfn); + ret = xc_mem_paging_load(xch, paging->mem_event.domain_id, gfn, + paging_buffer); if ( ret != 0 ) { if ( errno == ENOMEM )
@@ -662,33 +680,14 @@ static int xenpaging_populate_page(xenpasleep (1 );continue ; } -PERROR ("Error preparing %" PRI_xen_pfn" for page-in" , gfn); - goto out_map; +PERROR ("Error loading %" PRI_xen_pfn" during page-in" , gfn); + goto out; } }while ( ret && !interrupted ); -/* Map page */ - ret = -EFAULT ; - page =xc_map_foreign_pages (xch, paging->mem_event.domain_id, -PROT_READ |PROT_WRITE , &gfn,1 ); -if ( page ==NULL ) - { -PERROR ("Error mapping page %" PRI_xen_pfn": page is null" , gfn); - goto out_map; - } - -/* Read page */ - ret =read_page (fd, page, i); -if ( ret !=0 ) - { -PERROR ("Error reading page %" PRI_xen_pfn"" , gfn); - goto out; - } out: -munmap (page,PAGE_SIZE ); - out_map:return ret; }
Re: [PATCH 2 of 3] Tools: Libxc wrappers to automatically fill in page oud page contents on prepare
In reply to Andres Lagar-CavillaAndres Lagar-Cavilla writes ("[PATCH 2 of 3] Tools: Libxc wrappers to automatically fill in page oud page contents on prepare"):
Quoted 7 lines
tools/libxc/xc_mem_event.c | 4 ++-- tools/libxc/xc_mem_paging.c | 23 +++++++++++++++++++++++ tools/libxc/xenctrl.h | 2 ++ 3 files changed, 27 insertions(+), 2 deletions(-)Signed-off-by Andres Lagar-Cavilla <[email protected]>
Re: [PATCH 3 of 3] Teach xenpaging to use the new and non-racy xc_mem_paging_load
In reply to Andres Lagar-CavillaAndres Lagar-Cavilla writes ("[PATCH 3 of 3] Teach xenpaging to use the new and non-racy xc_mem_paging_load"):
Quoted 4 lines
tools/xenpaging/xenpaging.c | 43 +++++++++++++++++++++---------------------- 1 files changed, 21 insertions(+), 22 deletions(-)interface.
This one should get an ack from Olaf, probably.
Also your commit message seems to have been mangled.
Thanks,
Ian.
Re: [PATCH 1 of 3] After preparing a page for page-in, allow immediate fill-in of the page contents
In reply to Andres Lagar-CavillaQuoted 18 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/mem_event.c | 2 +- xen/arch/x86/mm/mem_paging.c | 2 +- xen/arch/x86/mm/p2m.c | 32 ++++++++++++++++++++++++++++++-- xen/include/asm-x86/p2m.h | 2 +- xen/include/public/domctl.h | 8 ++++++-- 5 files changed, 39 insertions(+), 7 deletions(-)
p2m_mem_paging_prepensures that an mfn is backing the paged-out gfn, and transitions to the next state in the paging state machine for that page. Foreign mappings of the gfn will now succeed. This is the key idea, as it allows the pager to now map the gfn and fill in its contents.Unfortunately, it also allows any other foreign mapper to map the gfn and read its contents. This is particularly dangerous when the populate is launched by a foreign mapper in the first place, which will be actively retrying the map operation and might race with the pager. Qemu-dm being a prime example.
Quoted 5 lines
Fix the race by allowing a buffer to be optionally passed in the prep operation, and having the hypervisor memcpy from that buffer into the newly prepped page before promoting the gfn type.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
Once Olaf OKs the xenpaging change, this whole set can go in.
Tim.
Re: [PATCH 3 of 3] Teach xenpaging to use the new and non-racy xc_mem_paging_load
In reply to Andres Lagar-CavillaQuoted 1 line · Andres Lagar-Cavilla
Signed-off-by Andres Lagar-Cavilla <[email protected]>
tools/xenpaging/xenpaging.c static char *dom_path; · line 48
Quoted 8 lines
diff -r
e71f880c0518-r90ded86b0a30tools/xenpaging/xenpaging.ctools/xenpaging/xenpaging.c +1
-0@@ -45,6 +45,7 @@ static char *dom_path;static char watch_token[16 ];static char *filename;static int interrupted; +static void *paging_buffer =NULL ;
Globals need no assignment.
Olaf
Tim Deegan
AppliedQuoted 14 lines · Andres Lagar-Cavilla
P2m_mem_paging_prepensures that an mfn is backing the paged-out gfn, and transitions to the next state in the paging state machine for this page. Foreign mappings of the gfn will now succeed. This is the key idea, as it allows the pager to now map the gfn and fill in its contents.Unfortunately, it also allows any other foreign mapper to map the gfn and read its contents. This is particularly dangerous when the populate is launched by a foreign mapper in the first place, which will be actively retrying the map operation and might race with the pager. Qemu-dm being a prime example.
Fix the race by allowing a buffer to be optionally passed in the prep operation, and having the hypervisor memcpy from that buffer into the newly prepped page before promoting the gfn type.
Quoted 7 lines
Second patch is a tools patch.
Resent after feedback: xenpaging patch attached, simplified with use of
copy_from_guest. Left potntial short-cut to avoidpging_resumefor further discussion.Signed-off-by Andres Lagar-Cavilla <[email protected]>
Applied, thanks.
Tim.