diff mbox

[v2,2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP

Message ID 1486736677-10953-3-git-send-email-paul.durrant@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Durrant Feb. 10, 2017, 2:24 p.m. UTC
Recently a new dm_op[1] hypercall was added to Xen to provide a mechanism
for restricting device emulators (such as QEMU) to a limited set of
hypervisor operations, and being able to audit those operations in the
kernel of the domain in which they run.

This patch adds IOCTL_PRIVCMD_DM_OP as gateway for __HYPERVISOR_dm_op,
bouncing the callers buffers through kernel memory to allow the address
ranges to be audited (and negating the need to bounce through locked
memory in user-space).

[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=524a98c2

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Juergen Gross <jgross@suse.com>

v2:
- Lock the user pages rather than bouncing through kernel memory
---
 arch/x86/include/asm/xen/hypercall.h |   7 ++
 drivers/xen/privcmd.c                | 138 +++++++++++++++++++++++++++++++++++
 include/uapi/xen/privcmd.h           |  13 ++++
 include/xen/interface/hvm/dm_op.h    |  32 ++++++++
 include/xen/interface/xen.h          |   1 +
 5 files changed, 191 insertions(+)
 create mode 100644 include/xen/interface/hvm/dm_op.h

Comments

Boris Ostrovsky Feb. 10, 2017, 4:17 p.m. UTC | #1
On 02/10/2017 09:24 AM, Paul Durrant wrote:
> +static long privcmd_ioctl_dm_op(void __user *udata)
> +{
> +	struct privcmd_dm_op kdata;
> +	struct privcmd_dm_op_buf *kbufs;
> +	unsigned int nr_pages = 0;
> +	struct page **pages = NULL;
> +	struct xen_dm_op_buf *xbufs = NULL;
> +	unsigned int i;
> +	long rc;
> +
> +	if (copy_from_user(&kdata, udata, sizeof(kdata)))
> +		return -EFAULT;
> +
> +	if (kdata.num == 0)
> +		return 0;
> +
> +	/*
> +	 * Set a tolerable upper limit on the number of buffers
> +	 * without being overly restrictive, since we can't easily
> +	 * predict what future dm_ops may require.
> +	 */

I think this deserves its own macro since it really has nothing to do
with page size, has it? Especially since you are referencing it again
below too.


> +	if (kdata.num * sizeof(*kbufs) > PAGE_SIZE)
> +		return -E2BIG;
> +
> +	kbufs = kcalloc(kdata.num, sizeof(*kbufs), GFP_KERNEL);
> +	if (!kbufs)
> +		return -ENOMEM;
> +
> +	if (copy_from_user(kbufs, kdata.ubufs,
> +			   sizeof(*kbufs) * kdata.num)) {
> +		rc = -EFAULT;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < kdata.num; i++) {
> +		if (!access_ok(VERIFY_WRITE, kbufs[i].uptr,
> +			       kbufs[i].size)) {
> +			rc = -EFAULT;
> +			goto out;
> +		}
> +
> +		nr_pages += DIV_ROUND_UP(
> +			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
> +			PAGE_SIZE);
> +	}
> +
> +	/*
> +	 * Again, set a tolerable upper limit on the number of pages
> +	 * needed to lock all the buffers without being overly
> +	 * restrictive, since we can't easily predict the size of
> +	 * buffers future dm_ops may use.
> +	 */

OTOH, these two cases describe different types of copying (the first one
is for buffer descriptors and the second is for buffers themselves). And
so should they be limited by the same value?

> +	if (nr_pages * sizeof(*pages) > PAGE_SIZE) {
> +		rc = -E2BIG;
> +		goto out;
> +	}
> +
> +	pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> +	if (!pages) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	xbufs = kcalloc(kdata.num, sizeof(*xbufs), GFP_KERNEL);
> +	if (!xbufs) {
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	rc = lock_pages(kbufs, kdata.num, pages, nr_pages);


Aren't those buffers already locked (as Andrew mentioned)? They are
mmapped with MAP_LOCKED.

And I also wonder whether we need to take rlimit(RLIMIT_MEMLOCK) into
account.

-boris
Paul Durrant Feb. 10, 2017, 4:28 p.m. UTC | #2
> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 10 February 2017 16:18
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgross@suse.com>
> Subject: Re: [PATCH v2 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
> 
> On 02/10/2017 09:24 AM, Paul Durrant wrote:
> > +static long privcmd_ioctl_dm_op(void __user *udata)
> > +{
> > +	struct privcmd_dm_op kdata;
> > +	struct privcmd_dm_op_buf *kbufs;
> > +	unsigned int nr_pages = 0;
> > +	struct page **pages = NULL;
> > +	struct xen_dm_op_buf *xbufs = NULL;
> > +	unsigned int i;
> > +	long rc;
> > +
> > +	if (copy_from_user(&kdata, udata, sizeof(kdata)))
> > +		return -EFAULT;
> > +
> > +	if (kdata.num == 0)
> > +		return 0;
> > +
> > +	/*
> > +	 * Set a tolerable upper limit on the number of buffers
> > +	 * without being overly restrictive, since we can't easily
> > +	 * predict what future dm_ops may require.
> > +	 */
> 
> I think this deserves its own macro since it really has nothing to do
> with page size, has it? Especially since you are referencing it again
> below too.
> 
> 
> > +	if (kdata.num * sizeof(*kbufs) > PAGE_SIZE)
> > +		return -E2BIG;
> > +
> > +	kbufs = kcalloc(kdata.num, sizeof(*kbufs), GFP_KERNEL);
> > +	if (!kbufs)
> > +		return -ENOMEM;
> > +
> > +	if (copy_from_user(kbufs, kdata.ubufs,
> > +			   sizeof(*kbufs) * kdata.num)) {
> > +		rc = -EFAULT;
> > +		goto out;
> > +	}
> > +
> > +	for (i = 0; i < kdata.num; i++) {
> > +		if (!access_ok(VERIFY_WRITE, kbufs[i].uptr,
> > +			       kbufs[i].size)) {
> > +			rc = -EFAULT;
> > +			goto out;
> > +		}
> > +
> > +		nr_pages += DIV_ROUND_UP(
> > +			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
> > +			PAGE_SIZE);
> > +	}
> > +
> > +	/*
> > +	 * Again, set a tolerable upper limit on the number of pages
> > +	 * needed to lock all the buffers without being overly
> > +	 * restrictive, since we can't easily predict the size of
> > +	 * buffers future dm_ops may use.
> > +	 */
> 
> OTOH, these two cases describe different types of copying (the first one
> is for buffer descriptors and the second is for buffers themselves). And
> so should they be limited by the same value?
> 

I think there needs to be some limit and limiting the allocation to a page was the best I came up with. Can you think of a better one?

> > +	if (nr_pages * sizeof(*pages) > PAGE_SIZE) {
> > +		rc = -E2BIG;
> > +		goto out;
> > +	}
> > +
> > +	pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
> > +	if (!pages) {
> > +		rc = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	xbufs = kcalloc(kdata.num, sizeof(*xbufs), GFP_KERNEL);
> > +	if (!xbufs) {
> > +		rc = -ENOMEM;
> > +		goto out;
> > +	}
> > +
> > +	rc = lock_pages(kbufs, kdata.num, pages, nr_pages);
> 
> 
> Aren't those buffers already locked (as Andrew mentioned)? They are
> mmapped with MAP_LOCKED.

No, they're not. The new libxendevicemodel code I have does not make any use of xencall or guest handles when privcmd supports the DM_OP ioctl, so the caller buffers will not be locked.

> 
> And I also wonder whether we need to take rlimit(RLIMIT_MEMLOCK) into
> account.
> 

Maybe. I'll look at that.

  Paul

> -boris
> 
>
Boris Ostrovsky Feb. 10, 2017, 5:44 p.m. UTC | #3
On 02/10/2017 11:28 AM, Paul Durrant wrote:
>> -----Original Message-----
>> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
>> Sent: 10 February 2017 16:18
>> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
>> linux-kernel@vger.kernel.org
>> Cc: Juergen Gross <jgross@suse.com>
>> Subject: Re: [PATCH v2 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
>>
>> On 02/10/2017 09:24 AM, Paul Durrant wrote:
>>> +static long privcmd_ioctl_dm_op(void __user *udata)
>>> +{
>>> +	struct privcmd_dm_op kdata;
>>> +	struct privcmd_dm_op_buf *kbufs;
>>> +	unsigned int nr_pages = 0;
>>> +	struct page **pages = NULL;
>>> +	struct xen_dm_op_buf *xbufs = NULL;
>>> +	unsigned int i;
>>> +	long rc;
>>> +
>>> +	if (copy_from_user(&kdata, udata, sizeof(kdata)))
>>> +		return -EFAULT;
>>> +
>>> +	if (kdata.num == 0)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * Set a tolerable upper limit on the number of buffers
>>> +	 * without being overly restrictive, since we can't easily
>>> +	 * predict what future dm_ops may require.
>>> +	 */
>> I think this deserves its own macro since it really has nothing to do
>> with page size, has it? Especially since you are referencing it again
>> below too.
>>
>>
>>> +	if (kdata.num * sizeof(*kbufs) > PAGE_SIZE)
>>> +		return -E2BIG;
>>> +
>>> +	kbufs = kcalloc(kdata.num, sizeof(*kbufs), GFP_KERNEL);
>>> +	if (!kbufs)
>>> +		return -ENOMEM;
>>> +
>>> +	if (copy_from_user(kbufs, kdata.ubufs,
>>> +			   sizeof(*kbufs) * kdata.num)) {
>>> +		rc = -EFAULT;
>>> +		goto out;
>>> +	}
>>> +
>>> +	for (i = 0; i < kdata.num; i++) {
>>> +		if (!access_ok(VERIFY_WRITE, kbufs[i].uptr,
>>> +			       kbufs[i].size)) {
>>> +			rc = -EFAULT;
>>> +			goto out;
>>> +		}
>>> +
>>> +		nr_pages += DIV_ROUND_UP(
>>> +			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
>>> +			PAGE_SIZE);
>>> +	}
>>> +
>>> +	/*
>>> +	 * Again, set a tolerable upper limit on the number of pages
>>> +	 * needed to lock all the buffers without being overly
>>> +	 * restrictive, since we can't easily predict the size of
>>> +	 * buffers future dm_ops may use.
>>> +	 */
>> OTOH, these two cases describe different types of copying (the first one
>> is for buffer descriptors and the second is for buffers themselves). And
>> so should they be limited by the same value?
>>
> I think there needs to be some limit and limiting the allocation to a page was the best I came up with. Can you think of a better one?

How about something like (with rather arbitrary values)

#define PRIVCMD_DMOP_MAX_NUM_BUFFERS       16
#define PRIVCMD_DMOP_MAX_TOT_BUFFER_SZ     4096

and make them part of the interface (i.e. put them into privcmd.h)?

-boris
kernel test robot Feb. 10, 2017, 7:25 p.m. UTC | #4
Hi Paul,

[auto build test ERROR on xen-tip/linux-next]
[also build test ERROR on v4.10-rc7 next-20170210]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Paul-Durrant/xen-privcmd-support-for-dm_op-and-restriction/20170211-001520
base:   https://git.kernel.org/pub/scm/linux/kernel/git/xen/tip.git linux-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/xen/privcmd.c: In function 'privcmd_ioctl_dm_op':
>> drivers/xen/privcmd.c:673:7: error: implicit declaration of function 'HYPERVISOR_dm_op' [-Werror=implicit-function-declaration]
     rc = HYPERVISOR_dm_op(kdata.dom, kdata.num, xbufs);
          ^~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/HYPERVISOR_dm_op +673 drivers/xen/privcmd.c

   667		for (i = 0; i < kdata.num; i++) {
   668			set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr);
   669			xbufs[i].size = kbufs[i].size;
   670		}
   671	
   672		xen_preemptible_hcall_begin();
 > 673		rc = HYPERVISOR_dm_op(kdata.dom, kdata.num, xbufs);
   674		xen_preemptible_hcall_end();
   675	
   676	out:

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Paul Durrant Feb. 13, 2017, 9:03 a.m. UTC | #5
> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 10 February 2017 17:45
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgross@suse.com>
> Subject: Re: [PATCH v2 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
> 
> On 02/10/2017 11:28 AM, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> >> Sent: 10 February 2017 16:18
> >> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-
> devel@lists.xenproject.org;
> >> linux-kernel@vger.kernel.org
> >> Cc: Juergen Gross <jgross@suse.com>
> >> Subject: Re: [PATCH v2 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
> >>
> >> On 02/10/2017 09:24 AM, Paul Durrant wrote:
> >>> +static long privcmd_ioctl_dm_op(void __user *udata)
> >>> +{
> >>> +	struct privcmd_dm_op kdata;
> >>> +	struct privcmd_dm_op_buf *kbufs;
> >>> +	unsigned int nr_pages = 0;
> >>> +	struct page **pages = NULL;
> >>> +	struct xen_dm_op_buf *xbufs = NULL;
> >>> +	unsigned int i;
> >>> +	long rc;
> >>> +
> >>> +	if (copy_from_user(&kdata, udata, sizeof(kdata)))
> >>> +		return -EFAULT;
> >>> +
> >>> +	if (kdata.num == 0)
> >>> +		return 0;
> >>> +
> >>> +	/*
> >>> +	 * Set a tolerable upper limit on the number of buffers
> >>> +	 * without being overly restrictive, since we can't easily
> >>> +	 * predict what future dm_ops may require.
> >>> +	 */
> >> I think this deserves its own macro since it really has nothing to do
> >> with page size, has it? Especially since you are referencing it again
> >> below too.
> >>
> >>
> >>> +	if (kdata.num * sizeof(*kbufs) > PAGE_SIZE)
> >>> +		return -E2BIG;
> >>> +
> >>> +	kbufs = kcalloc(kdata.num, sizeof(*kbufs), GFP_KERNEL);
> >>> +	if (!kbufs)
> >>> +		return -ENOMEM;
> >>> +
> >>> +	if (copy_from_user(kbufs, kdata.ubufs,
> >>> +			   sizeof(*kbufs) * kdata.num)) {
> >>> +		rc = -EFAULT;
> >>> +		goto out;
> >>> +	}
> >>> +
> >>> +	for (i = 0; i < kdata.num; i++) {
> >>> +		if (!access_ok(VERIFY_WRITE, kbufs[i].uptr,
> >>> +			       kbufs[i].size)) {
> >>> +			rc = -EFAULT;
> >>> +			goto out;
> >>> +		}
> >>> +
> >>> +		nr_pages += DIV_ROUND_UP(
> >>> +			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
> >>> +			PAGE_SIZE);
> >>> +	}
> >>> +
> >>> +	/*
> >>> +	 * Again, set a tolerable upper limit on the number of pages
> >>> +	 * needed to lock all the buffers without being overly
> >>> +	 * restrictive, since we can't easily predict the size of
> >>> +	 * buffers future dm_ops may use.
> >>> +	 */
> >> OTOH, these two cases describe different types of copying (the first one
> >> is for buffer descriptors and the second is for buffers themselves). And
> >> so should they be limited by the same value?
> >>
> > I think there needs to be some limit and limiting the allocation to a page
> was the best I came up with. Can you think of a better one?
> 
> How about something like (with rather arbitrary values)
> 
> #define PRIVCMD_DMOP_MAX_NUM_BUFFERS       16
> #define PRIVCMD_DMOP_MAX_TOT_BUFFER_SZ     4096
> 
> and make them part of the interface (i.e. put them into privcmd.h)?

Given that the values are arbitrary, I think it may be better to make them module params. They can then at least be tweaked if privcmd becomes a problem with later dm_ops.

  Paul

> 
> -boris
Boris Ostrovsky Feb. 13, 2017, 2:08 p.m. UTC | #6
>> How about something like (with rather arbitrary values)
>>
>> #define PRIVCMD_DMOP_MAX_NUM_BUFFERS       16
>> #define PRIVCMD_DMOP_MAX_TOT_BUFFER_SZ     4096
>>
>> and make them part of the interface (i.e. put them into privcmd.h)?
>
> Given that the values are arbitrary, I think it may be better to make them module params. They can then at least be tweaked if privcmd becomes a problem with later dm_ops.

Making them adjustable is a good thing but we still need default values.


-boris
Paul Durrant Feb. 13, 2017, 2:19 p.m. UTC | #7
> -----Original Message-----
> From: Boris Ostrovsky [mailto:boris.ostrovsky@oracle.com]
> Sent: 13 February 2017 14:09
> To: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> linux-kernel@vger.kernel.org
> Cc: Juergen Gross <jgross@suse.com>
> Subject: Re: [PATCH v2 2/3] xen/privcmd: Add IOCTL_PRIVCMD_DM_OP
> 
> 
> >> How about something like (with rather arbitrary values)
> >>
> >> #define PRIVCMD_DMOP_MAX_NUM_BUFFERS       16
> >> #define PRIVCMD_DMOP_MAX_TOT_BUFFER_SZ     4096
> >>
> >> and make them part of the interface (i.e. put them into privcmd.h)?
> >
> > Given that the values are arbitrary, I think it may be better to make them
> module params. They can then at least be tweaked if privcmd becomes a
> problem with later dm_ops.
> 
> Making them adjustable is a good thing but we still need default values.
> 

Oh, sure. I think I'll actually go for 16 bufs and a max of 4k per bufs as defaults. Patch v3 (including a fix for the arm build) coming shortly.

Cheers,

  Paul

> 
> -boris
diff mbox

Patch

diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index a12a047..f6d20f6 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -472,6 +472,13 @@  HYPERVISOR_xenpmu_op(unsigned int op, void *arg)
 	return _hypercall2(int, xenpmu_op, op, arg);
 }
 
+static inline int
+HYPERVISOR_dm_op(
+	domid_t dom, unsigned int nr_bufs, void *bufs)
+{
+	return _hypercall3(int, dm_op, dom, nr_bufs, bufs);
+}
+
 static inline void
 MULTI_fpu_taskswitch(struct multicall_entry *mcl, int set)
 {
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e5c7ae..d5cf042 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -32,6 +32,7 @@ 
 #include <xen/xen.h>
 #include <xen/privcmd.h>
 #include <xen/interface/xen.h>
+#include <xen/interface/hvm/dm_op.h>
 #include <xen/features.h>
 #include <xen/page.h>
 #include <xen/xen-ops.h>
@@ -548,6 +549,139 @@  static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
 	goto out;
 }
 
+static int lock_pages(
+	struct privcmd_dm_op_buf kbufs[], unsigned int num,
+	struct page *pages[], unsigned int nr_pages)
+{
+	unsigned int i;
+
+	for (i = 0; i < num; i++) {
+		unsigned int requested;
+		int pinned;
+
+		requested = DIV_ROUND_UP(
+			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
+			PAGE_SIZE);
+		if (requested > nr_pages)
+			return -ENOSPC;
+
+		pinned = get_user_pages_fast(
+			(unsigned long) kbufs[i].uptr,
+			requested, FOLL_WRITE, pages);
+		if (pinned < 0)
+			return pinned;
+
+		nr_pages -= pinned;
+		pages += pinned;
+	}
+
+	return 0;
+}
+
+static void unlock_pages(struct page *pages[], unsigned int nr_pages)
+{
+	unsigned int i;
+
+	if (!pages)
+		return;
+
+	for (i = 0; i < nr_pages; i++) {
+		if (pages[i])
+			put_page(pages[i]);
+	}
+}
+
+static long privcmd_ioctl_dm_op(void __user *udata)
+{
+	struct privcmd_dm_op kdata;
+	struct privcmd_dm_op_buf *kbufs;
+	unsigned int nr_pages = 0;
+	struct page **pages = NULL;
+	struct xen_dm_op_buf *xbufs = NULL;
+	unsigned int i;
+	long rc;
+
+	if (copy_from_user(&kdata, udata, sizeof(kdata)))
+		return -EFAULT;
+
+	if (kdata.num == 0)
+		return 0;
+
+	/*
+	 * Set a tolerable upper limit on the number of buffers
+	 * without being overly restrictive, since we can't easily
+	 * predict what future dm_ops may require.
+	 */
+	if (kdata.num * sizeof(*kbufs) > PAGE_SIZE)
+		return -E2BIG;
+
+	kbufs = kcalloc(kdata.num, sizeof(*kbufs), GFP_KERNEL);
+	if (!kbufs)
+		return -ENOMEM;
+
+	if (copy_from_user(kbufs, kdata.ubufs,
+			   sizeof(*kbufs) * kdata.num)) {
+		rc = -EFAULT;
+		goto out;
+	}
+
+	for (i = 0; i < kdata.num; i++) {
+		if (!access_ok(VERIFY_WRITE, kbufs[i].uptr,
+			       kbufs[i].size)) {
+			rc = -EFAULT;
+			goto out;
+		}
+
+		nr_pages += DIV_ROUND_UP(
+			offset_in_page(kbufs[i].uptr) + kbufs[i].size,
+			PAGE_SIZE);
+	}
+
+	/*
+	 * Again, set a tolerable upper limit on the number of pages
+	 * needed to lock all the buffers without being overly
+	 * restrictive, since we can't easily predict the size of
+	 * buffers future dm_ops may use.
+	 */
+	if (nr_pages * sizeof(*pages) > PAGE_SIZE) {
+		rc = -E2BIG;
+		goto out;
+	}
+
+	pages = kcalloc(nr_pages, sizeof(*pages), GFP_KERNEL);
+	if (!pages) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	xbufs = kcalloc(kdata.num, sizeof(*xbufs), GFP_KERNEL);
+	if (!xbufs) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	rc = lock_pages(kbufs, kdata.num, pages, nr_pages);
+	if (rc)
+		goto out;
+
+	for (i = 0; i < kdata.num; i++) {
+		set_xen_guest_handle(xbufs[i].h, kbufs[i].uptr);
+		xbufs[i].size = kbufs[i].size;
+	}
+
+	xen_preemptible_hcall_begin();
+	rc = HYPERVISOR_dm_op(kdata.dom, kdata.num, xbufs);
+	xen_preemptible_hcall_end();
+
+out:
+	unlock_pages(pages, nr_pages);
+	kfree(xbufs);
+	kfree(pages);
+	kfree(kbufs);
+
+	return rc;
+}
+
 static long privcmd_ioctl(struct file *file,
 			  unsigned int cmd, unsigned long data)
 {
@@ -571,6 +705,10 @@  static long privcmd_ioctl(struct file *file,
 		ret = privcmd_ioctl_mmap_batch(udata, 2);
 		break;
 
+	case IOCTL_PRIVCMD_DM_OP:
+		ret = privcmd_ioctl_dm_op(udata);
+		break;
+
 	default:
 		break;
 	}
diff --git a/include/uapi/xen/privcmd.h b/include/uapi/xen/privcmd.h
index 7ddeeda..f8c5d75 100644
--- a/include/uapi/xen/privcmd.h
+++ b/include/uapi/xen/privcmd.h
@@ -77,6 +77,17 @@  struct privcmd_mmapbatch_v2 {
 	int __user *err;  /* array of error codes */
 };
 
+struct privcmd_dm_op_buf {
+	void __user *uptr;
+	size_t size;
+};
+
+struct privcmd_dm_op {
+	domid_t dom;
+	__u16 num;
+	const struct privcmd_dm_op_buf __user *ubufs;
+};
+
 /*
  * @cmd: IOCTL_PRIVCMD_HYPERCALL
  * @arg: &privcmd_hypercall_t
@@ -98,5 +109,7 @@  struct privcmd_mmapbatch_v2 {
 	_IOC(_IOC_NONE, 'P', 3, sizeof(struct privcmd_mmapbatch))
 #define IOCTL_PRIVCMD_MMAPBATCH_V2				\
 	_IOC(_IOC_NONE, 'P', 4, sizeof(struct privcmd_mmapbatch_v2))
+#define IOCTL_PRIVCMD_DM_OP					\
+	_IOC(_IOC_NONE, 'P', 5, sizeof(struct privcmd_dm_op))
 
 #endif /* __LINUX_PUBLIC_PRIVCMD_H__ */
diff --git a/include/xen/interface/hvm/dm_op.h b/include/xen/interface/hvm/dm_op.h
new file mode 100644
index 0000000..ee9e480
--- /dev/null
+++ b/include/xen/interface/hvm/dm_op.h
@@ -0,0 +1,32 @@ 
+/*
+ * Copyright (c) 2016, Citrix Systems Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ */
+
+#ifndef __XEN_PUBLIC_HVM_DM_OP_H__
+#define __XEN_PUBLIC_HVM_DM_OP_H__
+
+struct xen_dm_op_buf {
+	GUEST_HANDLE(void) h;
+	xen_ulong_t size;
+};
+DEFINE_GUEST_HANDLE_STRUCT(xen_dm_op_buf);
+
+#endif /* __XEN_PUBLIC_HVM_DM_OP_H__ */
diff --git a/include/xen/interface/xen.h b/include/xen/interface/xen.h
index 1b0d189..4f4830e 100644
--- a/include/xen/interface/xen.h
+++ b/include/xen/interface/xen.h
@@ -81,6 +81,7 @@ 
 #define __HYPERVISOR_tmem_op              38
 #define __HYPERVISOR_xc_reserved_op       39 /* reserved for XenClient */
 #define __HYPERVISOR_xenpmu_op            40
+#define __HYPERVISOR_dm_op                41
 
 /* Architecture-specific hypercall definitions. */
 #define __HYPERVISOR_arch_0               48