Steve - I agree with you.
But do you think that the security issue should be handled by Xen or Xen should
leave it to the domains (as it does most of the times in other cases as well).
Venkat
-----Original Message-----
From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
Sent: Thursday, February 05, 2009 5:57 PM
To: Kumar, Venkat
Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [Xen-devel] GNTTAB_copy
> Let me explain the scenario.
>
> Before the copy happens, Xen checks a few things about the source grant entry
> and the destination grant entry.
>
> =============================================================================
> if ( src_is_gref )
> {
> gdprintk(XENLOG_INFO, "Entering Source Stuff \n");
> gdprintk(XENLOG_INFO, "Source gref is %d \n",op->source.u.ref);
> rc = __acquire_grant_for_copy(sd, op->source.u.ref, 1, &s_frame);
> if ( rc != GNTST_okay )
> goto error_out;
> have_s_grant = 1;
> gdprintk(XENLOG_INFO, "Exiting Source Stuff \n");
> }
>
> if ( dest_is_gref )
> {
> gdprintk(XENLOG_INFO, "Entering dest Stuff \n");
> gdprintk(XENLOG_INFO, "dest gref is %d \n",op->dest.u.ref);
> rc = __acquire_grant_for_copy(dd, op->dest.u.ref, 0, &d_frame);
> if ( rc != GNTST_okay )
> goto error_out;
> have_d_grant = 1;
> gdprintk(XENLOG_INFO, "Exiting dest Stuff \n");
> }
> ============================================================================
> The hunk that you are referring to has a check "scombo.shorts.domid
> != current->domain->domain_id". This condition gets executed once
> each while verifying the source and destination grant entry
> information. The union "Scombo" is nothing but the grant entry, so
> obviously one of the domains cannot be current domain, which means
> this check is avoiding a copy across domains.
I'm not sure I understand this last sentence. First, just to make
sure that we're talking about the same thing, I'm going to describe
what I think you're trying to do. You have three domains (let's call
them A, B, and C), with a source buffer in domain A and a destination
buffer in domain B. You want to issue a grant copy hypercall in
domain C to copy the contents of domain A's buffer into domain B's
buffer. Is that a fair summary?
If so, I'd expect the easiest solution would be for domain A to issue
domain C with a read-only grant of its buffer, and domain B to issue
domain C a writable grant of its buffer, and for domain C to then
issue the hypercall specifying the two grants as source and
destination operands. If you do it that way, I'd expect both scombos
to specify domain C, in which case the access control check will pass
and the hypercall will be allowed. Is that not what you're seeing?
> It's not necessary to take out the whole "if" statement (as in the
> patch) but it is mandatory to take out the "domainID" check to allow
> copy across domains.
The domid check is absolutely necessary to the whole grant tables
security model, though, so we can't just remove it.
(Unless, as I said before, you're willing to ignore potential security
issues in order to put together a proof-of-concept quickly.)
Steven.
> -----Original Message-----
> From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> Sent: Thursday, February 05, 2009 5:02 PM
> To: Kumar, Venkat
> Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] GNTTAB_copy
>
> > This patch is required even for a "copy via grant reference" mechanism.
> > Patch contains changes to support both gmfn and gref copies.
> I'd guess you're referring to this hunk?
>
> > *** /home/vduvvuru/xen-3.3.0/xen/common/grant_table.c 2008-12-19
> > 19:41:28.000000000 +0530
> > --- xen/common/grant_table.c 2009-02-02 21:38:23.000000000 +0530
> > ***************
> > *** 1300,1313 ****
> > for ( ; ; )
> > {
> > /* If not already pinned, check the grant domid and type. */
> > ! if ( !act->pin &&
> > (((scombo.shorts.flags & GTF_type_mask) !=
> > GTF_permit_access) ||
> > (scombo.shorts.domid != current->domain->domain_id)) )
> > PIN_FAIL(unlock_out, GNTST_general_error,
> > "Bad flags (%x) or dom (%d). (expected dom
> > %d)\n",
> > scombo.shorts.flags, scombo.shorts.domid,
> > current->domain->domain_id);
> >
> > new_scombo = scombo;
> > new_scombo.shorts.flags |= GTF_reading;
> > --- 1301,1317 ----
> > for ( ; ; )
> > {
> > /* If not already pinned, check the grant domid and type. */
> > ! /* if ( !act->pin &&
> > (((scombo.shorts.flags & GTF_type_mask) !=
> > GTF_permit_access) ||
> > (scombo.shorts.domid != current->domain->domain_id)) )
> > + {
> > + gdprintk(XENLOG_INFO, "Grant entry Info \n Domid=%d, Flags=%d,
> > Frame=%d\n",sha->domid,sha->flags,sha->frame);
> > PIN_FAIL(unlock_out, GNTST_general_error,
> > "Bad flags (%x) or dom (%d). (expected dom
> > %d)\n",
> > scombo.shorts.flags, scombo.shorts.domid,
> > current->domain->domain_id);
> > + }*/
> >
> > new_scombo = scombo;
> > new_scombo.shorts.flags |= GTF_reading;
> This effectively turns off access checking on grant references used in
> a copy operation, so that any domain is able to access any grant
> reference, regardless of who the granting domain originally granted
> access to. That's obviously not ideal from a security point of view.
>
> Are you sure you're setting up the grants correctly, and granting
> access to the right domain?
>
> Steven.
>
>
> > -----Original Message-----
> > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > Sent: Thursday, February 05, 2009 4:38 PM
> > To: Kumar, Venkat
> > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > Subject: Re: [Xen-devel] GNTTAB_copy
> >
> >
> > > Please find the patches attached with this email.
> > >
> > > The changes you are interested are in grant_table.patch
> > Ah, okay. It looks from the patch as if you're using the grant copy
> > hypercall to do direct MFN-to-MFN copies across a domain boundary,
> > without any actual grant references getting in the way. Is that
> > correct? If so, you may want to re-think your design; grant
> > references are Xen's principle access control mechanism for memory,
> > and if you're not using them then it's likely to be difficult to make
> > your design suitably secure.
> >
> > (Of course, if this is just a research prototype, you may decide you
> > don't care about such things, in which case best of luck to you.)
> >
> > Steven.
> >
> >
> > > -----Original Message-----
> > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > > Sent: Thursday, February 05, 2009 4:13 PM
> > > To: Kumar, Venkat
> > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > > Subject: Re: [Xen-devel] GNTTAB_copy
> > >
> > >
> > >
> > > > Atlast it worked out. I had to make some changes in Xen (it wasn't
> > >
> > > > allowing a copy across domains) to make it work apart from the patch
> > >
> > > > you sent.
> > >
> > > That's odd. Would you mind sending me your patch, please, so that I
> > >
> > > can see what's wrong with the existing implementation?
> > >
> > >
> > >
> > > Steven.
> > >
> > >
> > >
> > >
> > >
> > > > -----Original Message-----
> > >
> > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > >
> > > > Sent: Friday, January 30, 2009 10:04 PM
> > >
> > > > To: Kumar, Venkat
> > >
> > > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > >
> > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > >
> > > >
> > >
> > > > > Steve,
> > >
> > > > >
> > >
> > > > > I have ported the GNTTAB_copy part from netchannel2 xen-unstable to
> > > > > xen-unstable. But the hypercall is failing with a status -1.
> > >
> > > > >
> > >
> > > > > xm dmesg says
> > >
> > > > > ===========================================
> > >
> > > > > Bad flags (0) or dom (0). (expected dom 9)
> > >
> > > > > ===========================================
> > >
> > > > Okay, so Xen started processing the grant table operation, but found
> > >
> > > > that the grant entry in the granting domain's grant table was bad
> > >
> > > > (because it was full of zeroes). That probably indicates that it was
> > >
> > > > accessing completely the wrong gref.
> > >
> > > >
> > >
> > > > > I am granting the page with a flag ( GTF_reading | GTF_writing &
> > > > > GTF_permit_access ) but still it did not work.
> > >
> > > > >
> > >
> > > > > What could be wrong here?
> > >
> > > > I'm not sure. I'd suggest you put some printks in your guests (both
> > >
> > > > the granting one and the HVM one) to make sure that they agree on the
> > >
> > > > grant reference number, and some in Xen to make sure that it's reading
> > >
> > > > the hypercall arguments correctly.
> > >
> > > >
> > >
> > > > Steven.
> > >
> > > >
> > >
> > > >
> > >
> > > > > -----Original Message-----
> > >
> > > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > >
> > > > > Sent: Friday, January 30, 2009 4:52 PM
> > >
> > > > > To: Kumar, Venkat
> > >
> > > > > Cc: Steven Smith; xen-devel@xxxxxxxxxxxxxxxxxxx
> > >
> > > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > >
> > > > >
> > >
> > > > > That's unfortunate; the repository works fine for me.
> > >
> > > > >
> > >
> > > > > There aren't any other ways of downloading the entire repository, but
> > >
> > > > > the changes you need to make are pretty self-contained, so I've
> > >
> > > > > attached the relevant cset. You'll need to get rid of the
> > >
> > > > > GNTTABOP_set_version bits, but apart from that it should all be fairly
> > >
> > > > > obvious.
> > >
> > > > >
> > >
> > > > > Steven.
> > >
> > > > >
> > >
> > > > >
> > >
> > > > > > Steven - Thanks for the pointer.
> > >
> > > > > > I get an error while downloading the code.
> > >
> > > > > >
> > >
> > > > > > =======================================================================
> > >
> > > > > > hg clone
> > > > > > http://xenbits.xensource.com/ext/netchannel2/xen-unstable.hg
> > >
> > > > > > destination directory: xen-unstable.hg
> > >
> > > > > > requesting all changes
> > >
> > > > > > adding changesets
> > >
> > > > > > transaction abort!
> > >
> > > > > > rollback completed
> > >
> > > > > > abort: Connection reset by peer
> > >
> > > > > > =======================================================================
> > >
> > > > > >
> > >
> > > > > > Do you know any alternate download?
> > >
> > > > > >
> > >
> > > > > > Venkat
> > >
> > > > > >
> > >
> > > > > > -----Original Message-----
> > >
> > > > > > From: Steven Smith [mailto:steven.smith@xxxxxxxxxx]
> > >
> > > > > > Sent: Friday, January 30, 2009 4:18 PM
> > >
> > > > > > To: Kumar, Venkat
> > >
> > > > > > Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> > >
> > > > > > Subject: Re: [Xen-devel] GNTTAB_copy
> > >
> > > > > >
> > >
> > > > > > > Is GNTTAB_copy command in the Hypercall
> > > > > > > "HYPERVISOR_grant_table_op"
> > >
> > > > > > > supported between two HVM's?
> > >
> > > > > > It's not in xen-unstable, no, but it is if you use the netchannel2
> > >
> > > > > > hypervisor (available from
> > >
> > > > > > http://xenbits.xensource.com/ext/netchannel2/xen-unstable.hg). It'd
> > >
> > > > > > be pretty easy to cross-port, if you wanted to have a go at that.
> > > > > > The
> > >
> > > > > > interesting cset is this one:
> > >
> > > > > >
> > >
> > > > > > changeset: 19112:ea4b9c439ac3
> > >
> > > > > > user: Steven Smith <steven.smith@xxxxxxxxxxxxx>
> > >
> > > > > > date: Thu Jan 22 09:53:12 2009 +0000
> > >
> > > > > > files: xen/arch/x86/hvm/hvm.c xen/include/xen/hypercall.h
> > >
> > > > > > description:
> > >
> > > > > > Allow GNTABOP_copy to be used from HVM domains.
> > >
> > > > > >
> > >
> > > > > > Steven.
> >
> >
> >
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home