Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Jan Beulich]

>>> On 21.09.16 at 19:09,  wrote:
> On Wed, Sep 21, 2016 at 2:56 PM, Jan Beulich  wrote:
>> Again this looks like much clutter with little benefit to me, i.e. I'd
>> then rather go with the unmodified original proposal. That's largely
>> because nothing really enforces anyone to use the (disconnected)
>> xen_dmop_foo_entry type. I.e. it continues to be a matter of the
>> programmer's and the reviewers' attention/discipline.
> 
> But is "Must use hypercall dmop, subop foo with struct dmop_foo" any
> different than "Must use hypercall bar with struct bar"?
> 
> In theory there could be a mismatch between the struct libxc expected
> to use for a whole hypercall with the struct Xen expected to find for
> an entire hypercall. But in practice that never happens because we set
> up the call with the appropriate struct once and then never change it.

Yes.

> (That is, we may change the struct elements, but not the name.)  This
> seems to me like a fairly similar case.

I don't think so - what I'm talking about here is the equivalent of
"we may change the struct elements". Such changes would go
unnoticed by the compiler the respective pseudo-struct-element
is a handle.

But anyway - I think we've beaten this horse to death, and I'm the
only one worried about type issues here. Therefore I think we
should just stop this discussion and go with the proposed model.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[George Dunlap]

On Wed, Sep 21, 2016 at 2:56 PM, Jan Beulich  wrote:
> Again this looks like much clutter with little benefit to me, i.e. I'd
> then rather go with the unmodified original proposal. That's largely
> because nothing really enforces anyone to use the (disconnected)
> xen_dmop_foo_entry type. I.e. it continues to be a matter of the
> programmer's and the reviewers' attention/discipline.

But is "Must use hypercall dmop, subop foo with struct dmop_foo" any
different than "Must use hypercall bar with struct bar"?

In theory there could be a mismatch between the struct libxc expected
to use for a whole hypercall with the struct Xen expected to find for
an entire hypercall. But in practice that never happens because we set
up the call with the appropriate struct once and then never change it.
(That is, we may change the struct elements, but not the name.)  This
seems to me like a fairly similar case.

Note that I'm not arguing for the extra "clutter" per se, I just think
that it will be pretty effective in mitigating the type safety issue.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv) [and 1 more messages]"):
> On 21.09.16 at 15:24, <ian.jack...@eu.citrix.com> wrote:
> > Would this be enough of an improvement, for you, to be worth the
> > additional type name clutter etc. ?
> 
> Again this looks like much clutter with little benefit to me, i.e. I'd
> then rather go with the unmodified original proposal. That's largely
> because nothing really enforces anyone to use the (disconnected)
> xen_dmop_foo_entry type. I.e. it continues to be a matter of the
> programmer's and the reviewers' attention/discipline.

Fair enough.

Thanks for your opinions.

Regards,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Jan Beulich]

>>> On 21.09.16 at 15:24, <ian.jack...@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv) [and 1 more messages]"):
>> On 21.09.16 at 14:23, <ian.jack...@eu.citrix.com> wrote:
>> > But changes in the contents of the specific struct /will/ be spotted.
>> 
>> As long as it is a structure, yes. What about someone changing
>> uint64_t to xen_pfn_t?
> 
> You mean if someone changes one of the buffers from "array of
> uint64_t" to "array of xen_pfn_t", so that the ABI changes on some but
> not all architectures ?
> 
> We could declare a rule that a dmop buffer may contain only a struct
> or type name specific to that dmop (or an array of such), and must
> always be accessed through a copy to or from such a type.
> 
> The rule would mean that if we had a dmop which wanted to call, using
> one of the buffers, a function like this:
> 
>  int some_function(uint64_t *pfns, size_t n_pfns);
> 
> You'd have to write the dmop definition like this:
> 
>  typedef uint64_t xen_dmop_foo_entry;
> 
>  xen_dmop_foo_entry *pfns;
>  size_t n_pfns;
> 
>  ...allocate table somehow...
>  ret = copy_dm_buffer_from_guest(pfns,
>  sizeof(pfns[0])*n_pfns,
>  buffers, nr_buffers, 1);
>  ...
>  ret = some_function(pfns, n_pfns);
> 
> And similar code, again using xen_dmop_foo_entry. on the calling side.
> Then if the buffer entry type is changed to xen_pfn_t you change it
> to:
> 
>  typedef xen_pfn_t xen_dmop_foo_entry;
> 
> Now, unless the rest of the code is changed too, the compiler will
> warn that a xen_pfn_t* cannot be converted to a uint64_t* in the call
> to some_function.  (And likewise at the hypercall site in libxc.)
> 
> Would this be enough of an improvement, for you, to be worth the
> additional type name clutter etc. ?

Again this looks like much clutter with little benefit to me, i.e. I'd
then rather go with the unmodified original proposal. That's largely
because nothing really enforces anyone to use the (disconnected)
xen_dmop_foo_entry type. I.e. it continues to be a matter of the
programmer's and the reviewers' attention/discipline.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv) [and 1 more messages]"):
> On 21.09.16 at 14:23, <ian.jack...@eu.citrix.com> wrote:
> > But changes in the contents of the specific struct /will/ be spotted.
> 
> As long as it is a structure, yes. What about someone changing
> uint64_t to xen_pfn_t?

You mean if someone changes one of the buffers from "array of
uint64_t" to "array of xen_pfn_t", so that the ABI changes on some but
not all architectures ?

We could declare a rule that a dmop buffer may contain only a struct
or type name specific to that dmop (or an array of such), and must
always be accessed through a copy to or from such a type.

The rule would mean that if we had a dmop which wanted to call, using
one of the buffers, a function like this:

 int some_function(uint64_t *pfns, size_t n_pfns);

You'd have to write the dmop definition like this:

 typedef uint64_t xen_dmop_foo_entry;

 xen_dmop_foo_entry *pfns;
 size_t n_pfns;

 ...allocate table somehow...
 ret = copy_dm_buffer_from_guest(pfns,
 sizeof(pfns[0])*n_pfns,
 buffers, nr_buffers, 1);
 ...
 ret = some_function(pfns, n_pfns);

And similar code, again using xen_dmop_foo_entry. on the calling side.
Then if the buffer entry type is changed to xen_pfn_t you change it
to:

 typedef xen_pfn_t xen_dmop_foo_entry;

Now, unless the rest of the code is changed too, the compiler will
warn that a xen_pfn_t* cannot be converted to a uint64_t* in the call
to some_function.  (And likewise at the hypercall site in libxc.)

Would this be enough of an improvement, for you, to be worth the
additional type name clutter etc. ?


> > But I don't think it is really fair to criticise *the discussion
> > document* (which is what Jennifer's email was) on the grounds that it
> > ommitted to discuss a potential downside which its authors felt was
> > minor[1].
> 
> Hmm, then I'm sorry if this came over the wrong way. I didn't
> mean to criticize the document as a whole, or how it was written.

Thanks.

> >  The purpose of a discussion document is to put forward a
> > proposal and/or summarise previous discussion.  It is not required to
> > be either neutral or, indeed, comprehensive - and even so, I found
> > this one quite comprehensive.
> 
> Nevertheless I think such a document should be as honest as
> possible wrt downsides of the (by the author(s)) preferred of
> potentially multiple approaches.

I agree with that, although the word "honest" is rather tendentious.

>  If some aspect is deemed minor,
> I don't think it should be omitted (as then readers won't know
> whether the aspect was considered at all), but mentioned and
> stated to be believed to be minor.

I think there is a balance to be struck between mentioning every
possible consideration, however minor and remote, and making the
document concise and readable.

It will inevitably sometimes occur (as it did here) that some issue
that one person thought not worth mentioning, seems quite important to
another person.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Jan Beulich]

>>> On 21.09.16 at 14:23, <ian.jack...@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv)"):
>> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
>> > So the actual hypercall call sites are all in-tree, in libxc.  If the
>> > format of the struct used for one of these guest handle slots changes,
>> > the same struct definition from the Xen headers is used both in the
>> > hypervisor and in libxc, and any incompatibility will be detected.
>> 
>> Wait, no. The guest handle slots in Jenny's proposal are basically
>> typeless:
> ...
>> Each sub-op implies a certain type for each handle slot it actually
>> uses.
> 
> Yes.  But in practice each such slot will in practice be accessed by
> using copy_dm_buffer_from_guest (or whatever) to copy it to a struct.
> It is intended that that same struct type will be used by libxc to
> populate the buffer.
> 
> Now, it is true that the compiler cannot check that the _same type_ is
> used in both places.  So, as I say:
> 
>It's true that changes to the semantics of these slots (for example, a
>change of a slot from "array of struct X" to "one struct Y") will not
>be detected by the compiler.
> 
> But changes in the contents of the specific struct /will/ be spotted.

As long as it is a structure, yes. What about someone changing
uint64_t to xen_pfn_t?

>>  but I do think that it failed to point out this downside,
>> which imo moves the balance between the two proposals.
> 
> I'm afraid that I want to complain about this part of your approach to
> the thread, which I think is unduly harsh.
> 
> It is of course fair to point out a potential downside of the proposal,
> which wasn't clearly discussed in the discussion document.  And it is
> sensible for us all to consider that potential downside along with the
> others - as indeed I do above.
> 
> But I don't think it is really fair to criticise *the discussion
> document* (which is what Jennifer's email was) on the grounds that it
> ommitted to discuss a potential downside which its authors felt was
> minor[1].

Hmm, then I'm sorry if this came over the wrong way. I didn't
mean to criticize the document as a whole, or how it was written.

>  The purpose of a discussion document is to put forward a
> proposal and/or summarise previous discussion.  It is not required to
> be either neutral or, indeed, comprehensive - and even so, I found
> this one quite comprehensive.

Nevertheless I think such a document should be as honest as
possible wrt downsides of the (by the author(s)) preferred of
potentially multiple approaches. If some aspect is deemed minor,
I don't think it should be omitted (as then readers won't know
whether the aspect was considered at all), but mentioned and
stated to be believed to be minor.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv) [and 1 more messages]

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv)"):
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> > So the actual hypercall call sites are all in-tree, in libxc.  If the
> > format of the struct used for one of these guest handle slots changes,
> > the same struct definition from the Xen headers is used both in the
> > hypervisor and in libxc, and any incompatibility will be detected.
> 
> Wait, no. The guest handle slots in Jenny's proposal are basically
> typeless:
...
> Each sub-op implies a certain type for each handle slot it actually
> uses.

Yes.  But in practice each such slot will in practice be accessed by
using copy_dm_buffer_from_guest (or whatever) to copy it to a struct.
It is intended that that same struct type will be used by libxc to
populate the buffer.

Now, it is true that the compiler cannot check that the _same type_ is
used in both places.  So, as I say:

   It's true that changes to the semantics of these slots (for example, a
   change of a slot from "array of struct X" to "one struct Y") will not
   be detected by the compiler.

But changes in the contents of the specific struct /will/ be spotted.

For example, consider the struct device_model_op, which in Jennifer's
document is in slot 0.  If someone edits struct device_model_op to
change its contents, both libxc and Xen see the same modified struct.

The problem can only occur if someone accesses a slot with entirely
the wrong struct type, or by the wrong slot number.  In such a
situation at least in new code, the thing probably wouldn't work at
all.

And even if we had some kind of typesafe system, the typesafety still
wouldn't spot uncontrolled ABI changes which would silently break old
callers.  So the need for careful review of DMOP changes is still
present.

So overall I think this typesafety problem is fairly limited.

> And then I disagree with the general view taken here: The issue
> I see is not with caller of the libxc interfaces (it was always clear
> that it would be possible to have type safety at that layer), but
> between libxc as the consumer and Xen as the producer of the
> interface.

I think you have misunderstood me.  I was discussing precisely
typesafety of that interface.


> > But *all* ABI changes to the DMOP interface need to be accompanied by
> > changes to libxc to add compatibility code.  I think it will be fairly
> > straightforward to check, during review, that each DMOP change comes
> > with a corresponding libxc change.
> 
> Well, yes, we can only hope for that. The main risk is that someone
> doesn't update the other side at all, when a change to one side gets
> done which compiles fine at both ends, and perhaps doesn't even
> alter the public header.

As I say I think if people are making uncontrolled changes to this
area, we have ABI stability problems not covered by the guest handle
typesafety system.  If we wanted to sort that would probably need to
make a proper DSL for the ABI.


> > But if you do not agree, then how about hiding the slots with a macro
> > setup ?  Something like:
> >[...]
> 
> Well, that's exactly the kind of workaround I'd like to avoid.

Well, I think it's a lot of effort for not much gain.  But it's
certainly doable and if you think it worthwhile I'm prepared to try
it.


Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv)"):
> On 21.09.16 at 13:28, <george.dun...@citrix.com> wrote:
> > If someone feels strongly enough to Nack the other version, please say
> > so now; otherwise, I think Ian (since it seems like he'll be the one
> > implementing it) should make his best judgement based on the arguments
> > available and begin implementation.
> 
> No, I don't think I'd nack David's / Jenny's version (after all the type
> issue can be taken care off by sufficient discipline of both submitters
> and reviewers),

Thanks.

Personally I think David/Jennifer's version is better than the
previous proposal based on implicit or separate address limits.


>  but I do think that it failed to point out this downside,
> which imo moves the balance between the two proposals.

I'm afraid that I want to complain about this part of your approach to
the thread, which I think is unduly harsh.

It is of course fair to point out a potential downside of the proposal,
which wasn't clearly discussed in the discussion document.  And it is
sensible for us all to consider that potential downside along with the
others - as indeed I do above.

But I don't think it is really fair to criticise *the discussion
document* (which is what Jennifer's email was) on the grounds that it
ommitted to discuss a potential downside which its authors felt was
minor[1].  The pu

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 21.09.16 at 13:28,  wrote:
> I think the other slight bit of information missing from Jan at least
> (and perhaps also David) is the difference between their preference /
> recommendation and their requirement.
> 
> That is, if David and Jan each strongly recommend their own preferred
> method, but are willing to (perhaps grudingly) give Ack's to an
> implementaton of the other's method, then it's really down to the one
> doing the implementation to read the advice and make their own decision
> as best they can.
> 
> If someone feels strongly enough to Nack the other version, please say
> so now; otherwise, I think Ian (since it seems like he'll be the one
> implementing it) should make his best judgement based on the arguments
> available and begin implementation.

No, I don't think I'd nack David's / Jenny's version (after all the type
issue can be taken care off by sufficient discipline of both submitters
and reviewers), but I do think that it failed to point out this downside,
which imo moves the balance between the two proposals.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 21.09.16 at 13:21, <ian.jack...@eu.citrix.com> wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv)"):
>> On 13.09.16 at 18:07, <david.vra...@citrix.com> wrote:
>> > This is an direct result of the requirement that the privcmd driver does
>> > not know the types of the sub-ops themselves.  We can't have this
>> > requirement and type safety.  Which do we want?
>> 
>> Both, which the proposal utilizing side band information on VA
>> ranges allows for. (And in any event this to me clearly is an
>> aspect that would need to be mentioned in the disadvantages
>> section of the document.)
> 
> The side band information approach has the problem that it is easy to
> accidentally write hypervisor code which misses the
> additionally-required range check.
> 
> It might be possible to provide both strict type safety of the kind
> you're concerned about here, _and_ protection against missing access
> checks, but it's not very straightforward.
> 
> One approach to do that would be to augment the guest handle array
> proposal with a typesafe macro system for accessing the guest handle
> slots.
> 
> But I think George has a good argument as to why this is not needed:
> 
>   If most of the time the hypercalls are made by calling libxc functions,
>   and the libxc functions have types as arguments, then the end caller has
>   the same type safety.  We'll have to be careful inside the individual
>   libxc functions, but that should be fairly straightforward to do.  So
>   the places where we need to take extra care should be very localized.
> 
> We do not expect DMOP callers to make the hypercalls directly.  (They
> can't sensibly do so because the DMOP ABI is not stable - they need
> the assistance of the stability layer which we intend to provide in
> libxc.)
> 
> So the actual hypercall call sites are all in-tree, in libxc.  If the
> format of the struct used for one of these guest handle slots changes,
> the same struct definition from the Xen headers is used both in the
> hypervisor and in libxc, and any incompatibility will be detected.

Wait, no. The guest handle slots in Jenny's proposal are basically
typeless:

typedef struct dm_op_buffer {
 XEN_GUEST_HANDLE(void) h;
 size_t len;
} dm_op_buffer_t;

Each sub-op implies a certain type for each handle slot it actually
uses.

And then I disagree with the general view taken here: The issue
I see is not with caller of the libxc interfaces (it was always clear
that it would be possible to have type safety at that layer), but
between libxc as the consumer and Xen as the producer of the
interface.

> It's true that changes to the semantics of these slots (for example, a
> change of a slot from "array of struct X" to "one struct Y") will not
> be detected by the compiler.

Well, the "one" vs "array of" part can't normally be taken care of
by compiler type checking anyway, except that for the "one" case
we wouldn't normally use a handle in the first place.

> But *all* ABI changes to the DMOP interface need to be accompanied by
> changes to libxc to add compatibility code.  I think it will be fairly
> straightforward to check, during review, that each DMOP change comes
> with a corresponding libxc change.

Well, yes, we can only hope for that. The main risk is that someone
doesn't update the other side at all, when a change to one side gets
done which compiles fine at both ends, and perhaps doesn't even
alter the public header.

> But if you do not agree, then how about hiding the slots with a macro
> setup ?  Something like:
>[...]

Well, that's exactly the kind of workaround I'd like to avoid.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 21/09/16 12:21, Ian Jackson wrote:
> Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, 
> re qemu depriv)"):
>> On 13.09.16 at 18:07, <david.vra...@citrix.com> wrote:
>>> This is an direct result of the requirement that the privcmd driver does
>>> not know the types of the sub-ops themselves.  We can't have this
>>> requirement and type safety.  Which do we want?
>>
>> Both, which the proposal utilizing side band information on VA
>> ranges allows for. (And in any event this to me clearly is an
>> aspect that would need to be mentioned in the disadvantages
>> section of the document.)
> 
> The side band information approach has the problem that it is easy to
> accidentally write hypervisor code which misses the
> additionally-required range check.
> 
> It might be possible to provide both strict type safety of the kind
> you're concerned about here, _and_ protection against missing access
> checks, but it's not very straightforward.
> 
> One approach to do that would be to augment the guest handle array
> proposal with a typesafe macro system for accessing the guest handle
> slots.
> 
> But I think George has a good argument as to why this is not needed:
> 
>   If most of the time the hypercalls are made by calling libxc functions,
>   and the libxc functions have types as arguments, then the end caller has
>   the same type safety.  We'll have to be careful inside the individual
>   libxc functions, but that should be fairly straightforward to do.  So
>   the places where we need to take extra care should be very localized.
> 
> We do not expect DMOP callers to make the hypercalls directly.  (They
> can't sensibly do so because the DMOP ABI is not stable - they need
> the assistance of the stability layer which we intend to provide in
> libxc.)
> 
> So the actual hypercall call sites are all in-tree, in libxc.  If the
> format of the struct used for one of these guest handle slots changes,
> the same struct definition from the Xen headers is used both in the
> hypervisor and in libxc, and any incompatibility will be detected.
> 
> It's true that changes to the semantics of these slots (for example, a
> change of a slot from "array of struct X" to "one struct Y") will not
> be detected by the compiler.
> 
> But *all* ABI changes to the DMOP interface need to be accompanied by
> changes to libxc to add compatibility code.  I think it will be fairly
> straightforward to check, during review, that each DMOP change comes
> with a corresponding libxc change.
> 
> 
> But if you do not agree, then how about hiding the slots with a macro
> setup ?  Something like:
> 
> struct device_model_op {
>  uint32_t op;
>  union {
>   struct op_foo foo;
>   struct op_bar bar;
>   /* etc... */
>  } u;
> };
> 
> struct op_foo {
>some stuff;
> #define DMOP_GUESTHANDLES_foo(O,ONE,MANY) \
>ONE(O,  struct op_foo_big_block,   big_block) \
>MANY(O, struct op_foo_data_array_element,  data_array)
> };
> DMOP_DEFINE(foo)
> 
> 
> Supported (preceded by) something like this (many toothpicks elided
> for clarity and ease of editing):
> 
> /* dmop typesafe slot machinery: */
> 
> #define DMOP_DEFINE(opname) \
> enum {
> DMOP_GUESTHANDLES_##opname(DMOP__SLOT_INDEX,DMOP__SLOT_INDEX)
> };
> DMOP_GUESTHANDLES_##opname(DMOP__ACCESSORS_ONE,DMOP__ACCESSORS_MANY)
> 
> #define DMOP__SLOT_INDEX(O,T,N) DMOP__SLOT_INDEX__##O##_##N,
> 
> #define DMOP__ACCESSORS_ONE(O,T,N) \
> static inline int copy_dm_buffer_from_guest_##O##_##N(
> T *dst,
> const struct device_model_op *dmop /* accesses [nr_]buffers */)
> {
> return copy_dm_buffer_from_guest__raw
> ((void*)dst, sizeof(*dst), dmop,
>  DMOP__SLOT_INDEX__##O##_##N);
> }
> likewise copy_...to
> 
> #define DMOP__ACCESSORS_MANY(O,T,N) \
> static inline size_t get_dm_buffer_array_size_##O##_##N ... {
> calculation divides buffer size by type size;
> }
> static inline long copy_dm_buffer_from_guest_##O##_##N(
> T *dst, size_t nr_dst,
> const struct device_model_op *dmop /* accesses [nr_]buffers */)
> {
> 
> 
> Personally I don't think this is worth the effort.  But if you do I
> would be happy to turn this into something which actually compiles
> :-).
> 
> Thanks for your attention.

I think the other slight bit of information missing fr

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: [Xen-devel] Device model operation hypercall (DMOP, re 
qemu depriv)"):
> On 13.09.16 at 18:07, <david.vra...@citrix.com> wrote:
> > This is an direct result of the requirement that the privcmd driver does
> > not know the types of the sub-ops themselves.  We can't have this
> > requirement and type safety.  Which do we want?
> 
> Both, which the proposal utilizing side band information on VA
> ranges allows for. (And in any event this to me clearly is an
> aspect that would need to be mentioned in the disadvantages
> section of the document.)

The side band information approach has the problem that it is easy to
accidentally write hypervisor code which misses the
additionally-required range check.

It might be possible to provide both strict type safety of the kind
you're concerned about here, _and_ protection against missing access
checks, but it's not very straightforward.

One approach to do that would be to augment the guest handle array
proposal with a typesafe macro system for accessing the guest handle
slots.

But I think George has a good argument as to why this is not needed:

  If most of the time the hypercalls are made by calling libxc functions,
  and the libxc functions have types as arguments, then the end caller has
  the same type safety.  We'll have to be careful inside the individual
  libxc functions, but that should be fairly straightforward to do.  So
  the places where we need to take extra care should be very localized.

We do not expect DMOP callers to make the hypercalls directly.  (They
can't sensibly do so because the DMOP ABI is not stable - they need
the assistance of the stability layer which we intend to provide in
libxc.)

So the actual hypercall call sites are all in-tree, in libxc.  If the
format of the struct used for one of these guest handle slots changes,
the same struct definition from the Xen headers is used both in the
hypervisor and in libxc, and any incompatibility will be detected.

It's true that changes to the semantics of these slots (for example, a
change of a slot from "array of struct X" to "one struct Y") will not
be detected by the compiler.

But *all* ABI changes to the DMOP interface need to be accompanied by
changes to libxc to add compatibility code.  I think it will be fairly
straightforward to check, during review, that each DMOP change comes
with a corresponding libxc change.


But if you do not agree, then how about hiding the slots with a macro
setup ?  Something like:

struct device_model_op {
 uint32_t op;
 union {
  struct op_foo foo;
  struct op_bar bar;
  /* etc... */
 } u;
};

struct op_foo {
   some stuff;
#define DMOP_GUESTHANDLES_foo(O,ONE,MANY) \
   ONE(O,  struct op_foo_big_block,   big_block) \
   MANY(O, struct op_foo_data_array_element,  data_array)
};
DMOP_DEFINE(foo)


Supported (preceded by) something like this (many toothpicks elided
for clarity and ease of editing):

/* dmop typesafe slot machinery: */

#define DMOP_DEFINE(opname) \
enum {
DMOP_GUESTHANDLES_##opname(DMOP__SLOT_INDEX,DMOP__SLOT_INDEX)
};
DMOP_GUESTHANDLES_##opname(DMOP__ACCESSORS_ONE,DMOP__ACCESSORS_MANY)

#define DMOP__SLOT_INDEX(O,T,N) DMOP__SLOT_INDEX__##O##_##N,

#define DMOP__ACCESSORS_ONE(O,T,N) \
static inline int copy_dm_buffer_from_guest_##O##_##N(
T *dst,
const struct device_model_op *dmop /* accesses [nr_]buffers */)
{
return copy_dm_buffer_from_guest__raw
((void*)dst, sizeof(*dst), dmop,
 DMOP__SLOT_INDEX__##O##_##N);
}
likewise copy_...to

#define DMOP__ACCESSORS_MANY(O,T,N) \
static inline size_t get_dm_buffer_array_size_##O##_##N ... {
calculation divides buffer size by type size;
}
static inline long copy_dm_buffer_from_guest_##O##_##N(
T *dst, size_t nr_dst,
const struct device_model_op *dmop /* accesses [nr_]buffers */)
{


Personally I don't think this is worth the effort.  But if you do I
would be happy to turn this into something which actually compiles
:-).

Thanks for your attention.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 13.09.16 at 18:07,  wrote:
> On 12/09/16 15:32, Jan Beulich wrote:
> On 09.09.16 at 17:16,  wrote:
>>> The following code illustrates this idea:
>>>
>>> typedef struct dm_op_buffer {
>>>  XEN_GUEST_HANDLE(void) h;
>>>  size_t len;
>>> } dm_op_buffer_t;
>> 
>> This implies that we'll lose all type safety on the handles passed, as
>> is also emphasized by the use of raw_copy_from_guest() in the code
>> outline further down.
> 
> This is an direct result of the requirement that the privcmd driver does
> not know the types of the sub-ops themselves.  We can't have this
> requirement and type safety.  Which do we want?

Both, which the proposal utilizing side band information on VA
ranges allows for. (And in any event this to me clearly is an
aspect that would need to be mentioned in the disadvantages
section of the document.)

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[David Vrabel]

On 12/09/16 15:32, Jan Beulich wrote:
 On 09.09.16 at 17:16,  wrote:
>> The following code illustrates this idea:
>>
>> typedef struct dm_op_buffer {
>>  XEN_GUEST_HANDLE(void) h;
>>  size_t len;
>> } dm_op_buffer_t;
> 
> This implies that we'll lose all type safety on the handles passed, as
> is also emphasized by the use of raw_copy_from_guest() in the code
> outline further down.

This is an direct result of the requirement that the privcmd driver does
not know the types of the sub-ops themselves.  We can't have this
requirement and type safety.  Which do we want?

I would point out that Linux copy_from_user() and copy_to_user()
functions are not type safe and I'm not aware that this causes many
problems.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 13.09.16 at 12:37,  wrote:
> On 12/09/16 15:32, Jan Beulich wrote:
> On 09.09.16 at 17:16,  wrote:
>>> The following code illustrates this idea:
>>>
>>> typedef struct dm_op_buffer {
>>>  XEN_GUEST_HANDLE(void) h;
>>>  size_t len;
>>> } dm_op_buffer_t;
>> 
>> This implies that we'll lose all type safety on the handles passed, as
>> is also emphasized by the use of raw_copy_from_guest() in the code
>> outline further down.
> 
> If most of the time the hypercalls are made by calling libxc functions,
> and the libxc functions have types as arguments, then the end caller has
> the same type safety.  We'll have to be careful inside the individual
> libxc functions, but that should be fairly straightforward to do.  So
> the places where we need to take extra care should be very localized.

My main fear isn't so much to get it wrong the first time round, but
to break things down the road by not adjusting types in all needed
places at once (because the compiler doesn't tell the programmer).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 12/09/16 15:32, Jan Beulich wrote:
 On 09.09.16 at 17:16,  wrote:
>> The following code illustrates this idea:
>>
>> typedef struct dm_op_buffer {
>>  XEN_GUEST_HANDLE(void) h;
>>  size_t len;
>> } dm_op_buffer_t;
> 
> This implies that we'll lose all type safety on the handles passed, as
> is also emphasized by the use of raw_copy_from_guest() in the code
> outline further down.

If most of the time the hypercalls are made by calling libxc functions,
and the libxc functions have types as arguments, then the end caller has
the same type safety.  We'll have to be careful inside the individual
libxc functions, but that should be fairly straightforward to do.  So
the places where we need to take extra care should be very localized.

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 09.09.16 at 17:16,  wrote:
> The following code illustrates this idea:
> 
> typedef struct dm_op_buffer {
>  XEN_GUEST_HANDLE(void) h;
>  size_t len;
> } dm_op_buffer_t;

This implies that we'll lose all type safety on the handles passed, as
is also emphasized by the use of raw_copy_from_guest() in the code
outline further down.

> int
> HYPERVISOR_device_model_op(
>  domid_t domid,
>  unsigned int nr_buffers,
>  XEN_GUEST_HANDLE_PARAM(dm_op_buffer_t) buffers)

Along the same lines this will add implicit agreements (presumably
solely written out as comments, or maybe via manifest constants)
about which element of the buffer array has which meaning, quite
contrary to the otherwise enforced agreement (through use of
structure fields).

> int copy_dm_buffer_from_guest(
>  void *dst,/* Kernel destination buffer  */
>  size_t max_len,   /* Size of destination buffer */
>  XEN_GUEST_HANDLE_PARAM(dm_op_buffer_t) buffers,
>/* dm_op_buffers passed into DMOP */
>  unsigned int nr_buffers,  /* total number of dm_op_buffers  */
>  unsigned int idx) /* Index of buffer we require */

Looking at other hypercalls, it is not uncommon that arrays get
read element by element. While of course this function can be
extended suitably (perhaps with additional macro wrappers to
deal with the base types of such arrays), it will then become more
cumbersome to use, extending the "Minor stylistic issue" mentioned
in the disadvantages section further down.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 09/09/16 16:16, Jennifer Herbert wrote:
> The following code illustrates this idea:
> 
> typedef struct dm_op_buffer {
> XEN_GUEST_HANDLE(void) h;
> size_t len;
> } dm_op_buffer_t;
> 
> int
> HYPERVISOR_device_model_op(
> domid_t domid,
> unsigned int nr_buffers,
> XEN_GUEST_HANDLE_PARAM(dm_op_buffer_t) buffers)
> 
> @domid: the domain the hypercall operates on.
> @nr_buffers; the number of buffers in the @buffers array.
> 
> @buffers: an array of buffers.  @buffers[0] contains device_model_op - the
> structure describing the sub-op and its parameters. @buffers[1],
> @buffers[2]
> etc. may be used by a sub-op for passing additional buffers.
> 
> struct device_model_op {
> uint32_t op;
> union {
>  struct op_1 op1;
>  struct op_2 op2;
>  /* etc... */
> } u;
> };
> 
> It is forbidden for the above struct (device_model_op) to contain any
> guest handles - if they are needed, they should instead be in
> HYPERVISOR_device_model_op->buffers.

Sounds plausible to me.

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[David Vrabel]

On 09/09/16 16:16, Jennifer Herbert wrote:
> On 01/08/16 12:32, Ian Jackson wrote:
>> I think we need to introduce a new hypercall (which I will call DMOP
>> for now) which may augment or replace some of HVMCTL.  Let me explain:
>>
> 
> I believe the new 'DMOP' hypercall is a good idea,  but following on
> from discussions, I propose a revised design, which I present below.
> Please let me know what you think.
> 
> Thanks,
> Jenny.
> 
> 
> DMOP  (multi-buffer variant)
> 
[...]
> Advantages of this system, over previouse DMOP proposals:
> 
> 
> *  The validation of address ranges is easily done by the privcmd driver,
>using standard kernel mechanisms.  No need to get Xen thinking about
>guest memory layout, which it should be independent of, and potentially
>adding confusion.

+1.

The user address limit in Linux is a per-thread property, so trying to
pass this information to the hypervisor would require passing this
information in every dm_op, or switching the information on every task
switch.  The user address limit is also architecture specific, and would
require every arch to expose this via some new API.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jennifer Herbert]

On 01/08/16 12:32, Ian Jackson wrote:

I think we need to introduce a new hypercall (which I will call DMOP
for now) which may augment or replace some of HVMCTL.  Let me explain:



I believe the new 'DMOP' hypercall is a good idea,  but following on
from discussions, I propose a revised design, which I present below.
Please let me know what you think.

Thanks,
Jenny.


DMOP  (multi-buffer variant)



Introduction


A previous proposal for a 'DMOP' was put forward by Ian Jackson on the 1st
of August. This proposal seem very promising, however a problem was
identified with it, that this proposal addresses.

The aim of DMOP, as before, is to prevent a compromised device model from
compromising domains other then the one it is associated with. (And is
therefore likely already compromised)

The previous proposal adds a DMOP hypercall, for use by the device models,
which places the domain ID in a fixed place, within the calling args,
such that the priv driver can always find it, and not need to know any
further details about the particular DMOP in order to validate it against
previously set (vie ioctl) domain.

The problem occurs when you have a DMOP with references to other user memory
objects, such as with Track dirty VRAM (As used with the VGA buffer).
Is this case, the address of this other user object needs to be vetted,
to ensure it is not within a restricted address ranges, such as kernel
memory. The real problem comes down to how you would vet this address -
the idea place to do this is within the privcmd driver, since it would have
knowledge of the address space involved. However, with a principal goal
of DMOP to allow privcmd to be free from any knowledge of DMOP's sub ops,
it would have no way to identify any user buffer  addresses that need
checking.  The alternative of allowing the hypervisor to vet the address
is also problematic, since it has no knowledge of the guest memory layout.


The Design
--

As with the previous design, we provide a new restriction ioctl, which
takes a domid parameter.  After that restriction ioctl is called, the
privcmd driver will permit only DMOP hypercalls, and only with the
specified target domid.

In the previous design, a DMOP consisted of one buffer, containing all of
the DMOP parameters, which may include further explicit references to
more buffers.  In this design, an array of buffers with length is presented,
with the first one containing the DMOP parameters, which could implicitly
reference to further buffers from within in the array. Here, the only
user buffers passed, are that found with the array, and so all can be 
audited

from privcmd.  With the passing of the length of the buffers array, privcmd
does not need to know which DMOP it is to audit them.

If the hypervisor ends up with the wrong number of buffers, it can reject
the DMOP at that point.

The following code illustrates this idea:

typedef struct dm_op_buffer {
XEN_GUEST_HANDLE(void) h;
size_t len;
} dm_op_buffer_t;

int
HYPERVISOR_device_model_op(
domid_t domid,
unsigned int nr_buffers,
XEN_GUEST_HANDLE_PARAM(dm_op_buffer_t) buffers)

@domid: the domain the hypercall operates on.
@nr_buffers; the number of buffers in the @buffers array.

@buffers: an array of buffers.  @buffers[0] contains device_model_op - the
structure describing the sub-op and its parameters. @buffers[1], @buffers[2]
etc. may be used by a sub-op for passing additional buffers.

struct device_model_op {
uint32_t op;
union {
 struct op_1 op1;
 struct op_2 op2;
 /* etc... */
} u;
};

It is forbidden for the above struct (device_model_op) to contain any
guest handles - if they are needed, they should instead be in
HYPERVISOR_device_model_op->buffers.

Validation by privcmd driver
--

If the privcmd driver has been restricted to specific domain (using a
 new ioctl), when it received an op, it will:

1. Check hypercall is DMOP.

2. Check domid == restricted domid.

3. For each @nr_buffers in @buffers: Check @h and @len give a buffer
   wholey in the user space part of the virtual address space. (e.g.,
   on Linux use access_ok()).


Xen Implementation
--

Since a DMOP sub of may need to copy or return a buffer from the guest,
as well as the DMOP itself to git the initial buffer, functions for doing
this would be written as below.  Note that care is taken to prevent
damage from buffer under or over run situations.  If the DMOP is called
with insufficient buffers, zeros will be read, while extra is ignored.


int copy_dm_buffer_from_guest(
void *dst,/* Kernel destination buffer  */
size_t max_len,   /* Size of destination buffer */
XEN_GUEST_HANDLE_PARAM(dm_op_buffer_t) buffers,
  /* dm_op_buffers passed into DMOP */
unsigned int nr_buffers,  /* total number of dm_op_buffers  */
  

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Wei Liu]

On Tue, Aug 30, 2016 at 12:02:26PM +0100, Ian Jackson wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
> > Well, in a way. And then not: Initially I had thought no issue would
> > arise, until I came to realize the kernel memory corruption potential.
> > Question is whether now we're overlooking some other not
> > immediately obvious issue. The problem with auditing is that
> > generally you can only look for things you're aware of (or magically
> > become aware of while looking at the code). But I guess we should
> > just go ahead with the patterns we know of.
> 
> I think so, yes.  I will take a look at the interfaces, at least, to
> see if I can spot anything missing.  This will probably generate some
> more stupid questions...
> 
> So, then, is everyone now happy with the overall approach ?  That is,
> as I wrote up in:
>   Message-ID: <22464.10246.708893.563...@mariner.uk.xensource.com>
>   Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
>   Date: Fri, 26 Aug 2016 12:29:10 +0100
> 

It looks like a reasonable plan to me.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Stefano Stabellini]

On Tue, 30 Aug 2016, Ian Jackson wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
> > Well, in a way. And then not: Initially I had thought no issue would
> > arise, until I came to realize the kernel memory corruption potential.
> > Question is whether now we're overlooking some other not
> > immediately obvious issue. The problem with auditing is that
> > generally you can only look for things you're aware of (or magically
> > become aware of while looking at the code). But I guess we should
> > just go ahead with the patterns we know of.
> 
> I think so, yes.  I will take a look at the interfaces, at least, to
> see if I can spot anything missing.  This will probably generate some
> more stupid questions...
> 
> So, then, is everyone now happy with the overall approach ?  That is,
> as I wrote up in:
>   Message-ID: <22464.10246.708893.563...@mariner.uk.xensource.com>
>   Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
>   Date: Fri, 26 Aug 2016 12:29:10 +0100

I think it looks good

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> Well, in a way. And then not: Initially I had thought no issue would
> arise, until I came to realize the kernel memory corruption potential.
> Question is whether now we're overlooking some other not
> immediately obvious issue. The problem with auditing is that
> generally you can only look for things you're aware of (or magically
> become aware of while looking at the code). But I guess we should
> just go ahead with the patterns we know of.

I think so, yes.  I will take a look at the interfaces, at least, to
see if I can spot anything missing.  This will probably generate some
more stupid questions...

So, then, is everyone now happy with the overall approach ?  That is,
as I wrote up in:
  Message-ID: <22464.10246.708893.563...@mariner.uk.xensource.com>
  Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
  Date: Fri, 26 Aug 2016 12:29:10 +0100

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 26.08.16 at 16:35,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 26.08.16 at 13:38,  wrote:
>> > Another example would be a DMOP that takes (or returns) an event
>> > channel number in the calling domain.  This would be a problem because
>> > there would be nothing to stop qemu from messing about with evtchns
>> > which dom0 is using for other purposes (or conversely, there would be
>> > no way for the dom0 evtchn driver to know about the returned evtchn
>> > number and allow qemu to receive it).
>> 
>> Doesn't that follow the more general "mixing up own and target
>> domains" pattern, which is relatively easy to audit for?
> 
> Yes, as I understand what you mean by that pattern, indeed.
> 
>> > Another might be a DMOP that implicitly grants the target domain some
>> > of the calling domain's scheduling priority.  (I realise this is quite
>> > implausible from a scheduling API POV, but it gives an idea.)
>> > 
>> > Another example is that of course VCPU pool management and VCPU-PCPU
>> > pinning must not be available via DMOP.
>> > 
>> > (I write `qemu' here for brevity and clarity, but really I mean any
>> > DMOP caller which is supposed to be privileged for the target domain
>> > but not generally privileged.)
>> 
>> These all look rather contrived, especially keeping in mind that
>> what we mean to exclude right now are accidental violations of
>> the intended isolation. I.e. I think for all of those one would need
>> to go to some lengths to actually achieve the "goal", but they are
>> rather unlikely to be the result of a bug.
> 
> Right.
> 
> So I think this confirms your conclusion that this "audit" (ie,
> checking that there are problems in these kind of categories) won't be
> very difficult ?

Well, in a way. And then not: Initially I had thought no issue would
arise, until I came to realize the kernel memory corruption potential.
Question is whether now we're overlooking some other not
immediately obvious issue. The problem with auditing is that
generally you can only look for things you're aware of (or magically
become aware of while looking at the code). But I guess we should
just go ahead with the patterns we know of.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 26.08.16 at 13:38,  wrote:
> > Another example would be a DMOP that takes (or returns) an event
> > channel number in the calling domain.  This would be a problem because
> > there would be nothing to stop qemu from messing about with evtchns
> > which dom0 is using for other purposes (or conversely, there would be
> > no way for the dom0 evtchn driver to know about the returned evtchn
> > number and allow qemu to receive it).
> 
> Doesn't that follow the more general "mixing up own and target
> domains" pattern, which is relatively easy to audit for?

Yes, as I understand what you mean by that pattern, indeed.

> > Another might be a DMOP that implicitly grants the target domain some
> > of the calling domain's scheduling priority.  (I realise this is quite
> > implausible from a scheduling API POV, but it gives an idea.)
> > 
> > Another example is that of course VCPU pool management and VCPU-PCPU
> > pinning must not be available via DMOP.
> > 
> > (I write `qemu' here for brevity and clarity, but really I mean any
> > DMOP caller which is supposed to be privileged for the target domain
> > but not generally privileged.)
> 
> These all look rather contrived, especially keeping in mind that
> what we mean to exclude right now are accidental violations of
> the intended isolation. I.e. I think for all of those one would need
> to go to some lengths to actually achieve the "goal", but they are
> rather unlikely to be the result of a bug.

Right.

So I think this confirms your conclusion that this "audit" (ie,
checking that there are problems in these kind of categories) won't be
very difficult ?

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 26.08.16 at 13:29,  wrote:
> Is this plan a good one for everyone ?

Sounds reasonable to me; just needs settling on a few of the actual
details.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 26.08.16 at 13:38,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 08.08.16 at 15:46,  wrote:
>> > So would it therefore be OK to introduce the enhanced security promise
>> > - the lack of `class 2' bugs - for HVMCTL from the beginning ?
>> 
>> I think so, since ...
>> 
>> > This would involve a small amount of extra thought for each invididual
>> > hypercall, just to check that the assumptions we are relying on (as
>> > you put them above) are not violated.
>> 
>> ... this looks to be a manageable amount of code auditing (albeit
>> I'd like to see whether someone else can perhaps come up with
>> some potential, more realistic kind of bug that could fall into class
>> 2 before volunteering to make an attempt at doing such an audit).
> 
> Right.
> 
> Let me try to think of some examples.  Thinking `aloud':
> 
> The real problem comes if a DMOP talks about the calling domain's
> resources or namespaces, implicitly or explicitly.
> 
> An example of an explicit reference to the calling domain's resources
> is the references to memory space in the calling domain (vaddrs).  We
> have already had an extensive discussion of that...
> 
> Another example would be a DMOP that takes (or returns) an event
> channel number in the calling domain.  This would be a problem because
> there would be nothing to stop qemu from messing about with evtchns
> which dom0 is using for other purposes (or conversely, there would be
> no way for the dom0 evtchn driver to know about the returned evtchn
> number and allow qemu to receive it).

Doesn't that follow the more general "mixing up own and target
domains" pattern, which is relatively easy to audit for?

> Another might be a DMOP that implicitly grants the target domain some
> of the calling domain's scheduling priority.  (I realise this is quite
> implausible from a scheduling API POV, but it gives an idea.)
> 
> Another example is that of course VCPU pool management and VCPU-PCPU
> pinning must not be available via DMOP.
> 
> (I write `qemu' here for brevity and clarity, but really I mean any
> DMOP caller which is supposed to be privileged for the target domain
> but not generally privileged.)

These all look rather contrived, especially keeping in mind that
what we mean to exclude right now are accidental violations of
the intended isolation. I.e. I think for all of those one would need
to go to some lengths to actually achieve the "goal", but they are
rather unlikely to be the result of a bug.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 08.08.16 at 15:46,  wrote:
> > So would it therefore be OK to introduce the enhanced security promise
> > - the lack of `class 2' bugs - for HVMCTL from the beginning ?
> 
> I think so, since ...
> 
> > This would involve a small amount of extra thought for each invididual
> > hypercall, just to check that the assumptions we are relying on (as
> > you put them above) are not violated.
> 
> ... this looks to be a manageable amount of code auditing (albeit
> I'd like to see whether someone else can perhaps come up with
> some potential, more realistic kind of bug that could fall into class
> 2 before volunteering to make an attempt at doing such an audit).

Right.

Let me try to think of some examples.  Thinking `aloud':

The real problem comes if a DMOP talks about the calling domain's
resources or namespaces, implicitly or explicitly.

An example of an explicit reference to the calling domain's resources
is the references to memory space in the calling domain (vaddrs).  We
have already had an extensive discussion of that...

Another example would be a DMOP that takes (or returns) an event
channel number in the calling domain.  This would be a problem because
there would be nothing to stop qemu from messing about with evtchns
which dom0 is using for other purposes (or conversely, there would be
no way for the dom0 evtchn driver to know about the returned evtchn
number and allow qemu to receive it).

Another might be a DMOP that implicitly grants the target domain some
of the calling domain's scheduling priority.  (I realise this is quite
implausible from a scheduling API POV, but it gives an idea.)

Another example is that of course VCPU pool management and VCPU-PCPU
pinning must not be available via DMOP.

(I write `qemu' here for brevity and clarity, but really I mean any
DMOP caller which is supposed to be privileged for the target domain
but not generally privileged.)

Does that help at all ?

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 15.08.16 at 16:50,  wrote:
> > It seems simpler to me to have in the privcmd driver:
> > 
> > if (op == HVMCTL_track_dirty_vram)
> > ret = access_ok(...);
> > 
> > It looks simpler to me to fix the ABI and add the checking in the
> > privcmd driver than add one of the proposed mechanisms to allow the
> > hypervisor to do the checking.
> 
> Simpler, yes, but ...
> 
> > To avoid the issues with having to update the kernel in lock-step with
> > the hypervisor (if new checks are needed in privcmd), we can (in the
> > common case) do version the checking in the driver.
> > 
> > i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
> > V+1 then it can choose to disable the depriv and thus continue to work
> > (with reduced defense in depth).
> 
> ... less flexible, and prone to end up in a mess once we have more
> than a handful of versions for the driver to deal with.

I agree.

To summarise the current proposal:

DMOP is roughly the same as Jan's HVMCTL.  It needs two
enhancements compared to HVMCTL:

 * Each hypercall transferred to DMOP[1] must promise not to do bad
   things to the calling domain (or to the whole system).  There will
   need to be some minimal audit (or consideration of the hypercall's
   functionality) for this purpose.  Jan thinks this is not too much
   work but wants an example or two of a plausible vulnerability that
   needs to be excluded.

 * We will need to enable the privcmd driver to defend dom0's memory.

   This will be done by: the privcmd driver specifying to the
   hypervisor (either inside each DMOP hypercall struct, or in a
   previous hypercall) a single address range which is permitted for
   this purpose.  (This would be the vaddr range which is used for
   userspace.)

   If the range is specified separately, the DMOP hypercall needs a
   flag to say whether this specific DMOP call ought to be subject to
   the restriction.


We also discussed compatibility.  We need to present a stable ABI and
API to qemu.  We considered making a limited stability promise for the
DMOP hypercalls, but this would involve the hypervisor supporting
multiple old versions of DMOPs which is not ideal from a security
point of view.  Instead, it is proposed that this will be dealt with
in libxc.

In more detail, I think the plan has then to look something like this:

Suppose a DMOP is introduced in Xen version K, and then modified in a
non-ABI-compatible way Xen version K+1.

Xen K+1 implements only the new ABI, and Xen K provides only the old.
libxc in K provides only the single entrypoint for the old DMOP.

libxc in K+1 provides a new entrypoint for the new DMOP (with enhanced
parameters, or whatever).  This new entrypoint needs no compatibility
handling and works only only Xen >= K+1 (and otherwise returns
ENOSYS).

However, libxc K+1 it also provides the old entrypoint.  This old
entrypoint checks the hypervisor DMOP ABI version, and makes either
the old or new hypercall.

For this to work, the ABI definition for the retired DMOP must remain
in xen.git somewhere, so that the code in libxc can call it.

Also, there should be a single DMOP ABI version which can be retrieved
by the hypervisor and cached by libxc.  This could be the hypervisor
ABI version if we don't mind breakage during development.


Is this plan a good one for everyone ?

Thanks,
Ian.


[1] By this I don't mean to take a point of view about the name.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 16:50,  wrote:
> On 09/08/16 11:29, Jan Beulich wrote:
> On 08.08.16 at 15:46,  wrote:
>>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>>> depriv)"):
 On 05.08.16 at 18:28,  wrote:
> That is, a bug of class 2 would allow the unprivileged qemu process in
> dom0 to cause damage to other parts of dom0.
>>> ...
 Ah, okay, I think I finally understand. [...]

 I'm having, however, a hard time imagining a class 2 bug for any
 of the hvmop-s that are being converted by the hvmctl series:
 These act on the target domain, so would not touch the calling
 ones state other than for copying argument structures to/from
 guest memory (and I don't view mixing up domain pointers as
 a likely source of problems).
>>>
>>> Right.  AIUI all the hypercall arguments are passed using
>>> calling-guest-virtual addresses, which the dom0 privcmd arrangements
>>> are capable of ensuring are sane.
>> 
>> Actually, having thought about this some more, and taking this
>> together with the expectations to the privcmd driver previously
>> outlined, I think this part is problematic: If all the driver is to know
>> is the position (within the interface structure) of the target domain
>> ID, then any guest handles embedded in the interface structure
>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>> validated, and hence user mode code would have a way to access
>> or modify kernel memory.
> 
> It seems simpler to me to have in the privcmd driver:
> 
> if (op == HVMCTL_track_dirty_vram)
> ret = access_ok(...);
> 
> It looks simpler to me to fix the ABI and add the checking in the
> privcmd driver than add one of the proposed mechanisms to allow the
> hypervisor to do the checking.

Simpler, yes, but ...

> To avoid the issues with having to update the kernel in lock-step with
> the hypervisor (if new checks are needed in privcmd), we can (in the
> common case) do version the checking in the driver.
> 
> i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
> V+1 then it can choose to disable the depriv and thus continue to work
> (with reduced defense in depth).

... less flexible, and prone to end up in a mess once we have more
than a handful of versions for the driver to deal with.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 16:57,  wrote:
> On 15/08/16 12:20, Jan Beulich wrote:
> On 15.08.16 at 12:47,  wrote:
>>> On 15/08/16 11:19, Jan Beulich wrote:
 Well, none of the options considered so far are really nice or
 readily available. I think the easiest to use for both the caller and
 the implementation of the hypercall would be the auxiliary
 hypercall for a kernel to indicate user/kernel boundaries plus a
 flag on the DMOP one for the kernel mode driver to indicate its
 user mode origin. The main (purely theoretical afaict) downside
 of this is the difficulty to use it in OSes with variable user/kernel
 boundaries.
>>>
>>> What about including in the "fixed" part of the hypercall a virtual
>>> address range that all pointers must be in?  That wouldn't even require
>>> a user/kernel flag actually; and could conceivably be used by the caller
>>> (either userspace or kernel space) to thwart certain kinds of potential
>>> attacks.
>> 
>> That's definitely an option, if we're sufficiently certain that no OSes
>> will ever require two or more ranges.
> 
> I'm sorry, I think this is getting a bit silly.  There are currently no
> known operating systems which have discontinuous user-space virtual
> address ranges.  Even if there were, the hypercall structure I'm
> proposing would still function; the only restriction would be that any
> single hypercall would have to have all arguments within one of those
> ranges.

Well, you then discount the original vDSO range 64-bit Linux had
high up in the fixmap area. Not that this area would be usable for
any bad here, but it shows that such an idea isn't pure theory.

> Another idea I had was to do a multi-call-style hypercall "wrapper"
> hypercall, similar to David's XSM idea, but instead of running with a
> specific XSM label, would restrict the target domain and VA range of all
> included hypercalls.  Then the individual hypercall structure wouldn't
> need to be known at all; and if it ever became important to have (say)
> two VA ranges, we could simply add a new "wrapper" hypercall.

That should work too, yes.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 15/08/16 12:20, Jan Beulich wrote:
 On 15.08.16 at 12:47,  wrote:
>> On 15/08/16 11:19, Jan Beulich wrote:
>>> Well, none of the options considered so far are really nice or
>>> readily available. I think the easiest to use for both the caller and
>>> the implementation of the hypercall would be the auxiliary
>>> hypercall for a kernel to indicate user/kernel boundaries plus a
>>> flag on the DMOP one for the kernel mode driver to indicate its
>>> user mode origin. The main (purely theoretical afaict) downside
>>> of this is the difficulty to use it in OSes with variable user/kernel
>>> boundaries.
>>
>> What about including in the "fixed" part of the hypercall a virtual
>> address range that all pointers must be in?  That wouldn't even require
>> a user/kernel flag actually; and could conceivably be used by the caller
>> (either userspace or kernel space) to thwart certain kinds of potential
>> attacks.
> 
> That's definitely an option, if we're sufficiently certain that no OSes
> will ever require two or more ranges.

I'm sorry, I think this is getting a bit silly.  There are currently no
known operating systems which have discontinuous user-space virtual
address ranges.  Even if there were, the hypercall structure I'm
proposing would still function; the only restriction would be that any
single hypercall would have to have all arguments within one of those
ranges.

If we find such a monster in the future, we can try to figure out how to
accommodate it at that point.

Another idea I had was to do a multi-call-style hypercall "wrapper"
hypercall, similar to David's XSM idea, but instead of running with a
specific XSM label, would restrict the target domain and VA range of all
included hypercalls.  Then the individual hypercall structure wouldn't
need to be known at all; and if it ever became important to have (say)
two VA ranges, we could simply add a new "wrapper" hypercall.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[David Vrabel]

On 09/08/16 11:29, Jan Beulich wrote:
 On 08.08.16 at 15:46,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> On 05.08.16 at 18:28,  wrote:
 That is, a bug of class 2 would allow the unprivileged qemu process in
 dom0 to cause damage to other parts of dom0.
>> ...
>>> Ah, okay, I think I finally understand. [...]
>>>
>>> I'm having, however, a hard time imagining a class 2 bug for any
>>> of the hvmop-s that are being converted by the hvmctl series:
>>> These act on the target domain, so would not touch the calling
>>> ones state other than for copying argument structures to/from
>>> guest memory (and I don't view mixing up domain pointers as
>>> a likely source of problems).
>>
>> Right.  AIUI all the hypercall arguments are passed using
>> calling-guest-virtual addresses, which the dom0 privcmd arrangements
>> are capable of ensuring are sane.
> 
> Actually, having thought about this some more, and taking this
> together with the expectations to the privcmd driver previously
> outlined, I think this part is problematic: If all the driver is to know
> is the position (within the interface structure) of the target domain
> ID, then any guest handles embedded in the interface structure
> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
> validated, and hence user mode code would have a way to access
> or modify kernel memory.

It seems simpler to me to have in the privcmd driver:

if (op == HVMCTL_track_dirty_vram)
ret = access_ok(...);

It looks simpler to me to fix the ABI and add the checking in the
privcmd driver than add one of the proposed mechanisms to allow the
hypervisor to do the checking.

To avoid the issues with having to update the kernel in lock-step with
the hypervisor (if new checks are needed in privcmd), we can (in the
common case) do version the checking in the driver.

i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
V+1 then it can choose to disable the depriv and thus continue to work
(with reduced defense in depth).

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 14:07,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 15.08.16 at 12:47,  wrote:
>> > What about including in the "fixed" part of the hypercall a virtual
>> > address range that all pointers must be in?  That wouldn't even require
>> > a user/kernel flag actually; and could conceivably be used by the caller
>> > (either userspace or kernel space) to thwart certain kinds of potential
>> > attacks.
>> 
>> That's definitely an option, if we're sufficiently certain that no OSes
>> will ever require two or more ranges.
> 
> How hard would it be to allow the caller to specify several allowable
> ranges ?

Not very hard - just like with so many things an additional level
of indirection would do.

> Note that the hypercall argument construction code in libxc already
> has to handle all hypercall argument memory specially, so libxc could
> automatically build a list of the arguments' memory addresses.
> 
> What would be needed is some kind of restriction on (or variant of)
> copy_* which double-checked against the list provided in the
> non-op-specific part of the hypercall.

Yeah, as George already mentioned. I'd favor "variant of", to avoid
penalizing all other callers.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 15.08.16 at 12:47,  wrote:
> > What about including in the "fixed" part of the hypercall a virtual
> > address range that all pointers must be in?  That wouldn't even require
> > a user/kernel flag actually; and could conceivably be used by the caller
> > (either userspace or kernel space) to thwart certain kinds of potential
> > attacks.
> 
> That's definitely an option, if we're sufficiently certain that no OSes
> will ever require two or more ranges.

How hard would it be to allow the caller to specify several allowable
ranges ?

Note that the hypercall argument construction code in libxc already
has to handle all hypercall argument memory specially, so libxc could
automatically build a list of the arguments' memory addresses.

What would be needed is some kind of restriction on (or variant of)
copy_* which double-checked against the list provided in the
non-op-specific part of the hypercall.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 12:47,  wrote:
> On 15/08/16 11:19, Jan Beulich wrote:
>> Well, none of the options considered so far are really nice or
>> readily available. I think the easiest to use for both the caller and
>> the implementation of the hypercall would be the auxiliary
>> hypercall for a kernel to indicate user/kernel boundaries plus a
>> flag on the DMOP one for the kernel mode driver to indicate its
>> user mode origin. The main (purely theoretical afaict) downside
>> of this is the difficulty to use it in OSes with variable user/kernel
>> boundaries.
> 
> What about including in the "fixed" part of the hypercall a virtual
> address range that all pointers must be in?  That wouldn't even require
> a user/kernel flag actually; and could conceivably be used by the caller
> (either userspace or kernel space) to thwart certain kinds of potential
> attacks.

That's definitely an option, if we're sufficiently certain that no OSes
will ever require two or more ranges.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 15/08/16 11:19, Jan Beulich wrote:
 On 15.08.16 at 11:39,  wrote:
>> On 12/08/16 12:50, Jan Beulich wrote:
>> On 12.08.16 at 11:44,  wrote:
 On 09/08/16 12:30, Jan Beulich wrote:
 On 09.08.16 at 12:48,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> Actually, having thought about this some more, and taking this
>>> together with the expectations to the privcmd driver previously
>>> outlined, I think this part is problematic: If all the driver is to know
>>> is the position (within the interface structure) of the target domain
>>> ID, then any guest handles embedded in the interface structure
>>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>>> validated, and hence user mode code would have a way to access
>>> or modify kernel memory.
>>
>> Could the hypervisor know the difference between user and kernel
>> memory, in principle ?
>
> Not without further new hypercalls, as the guest kernel would need
> to tell Xen what address ranges are kernel vs user (and that implies
> that any OS wishing to be able to act as Dom0 has a uniform
> separation of address spaces).

 Couldn't Xen tell from the guest pagetables whether the memory being
 accessed was user-mode or kernel mode?
>>>
>>> That would be possible, but would feel like adding heuristics instead
>>> of a proper distinction. Clearly we'd already be in some trouble if
>>> there were cases where some structure doesn't get written to (and
>>> hence could live in user-r/o mapped space), but others would need
>>> to be verified to be user-r/w mapped. A lot of special casing, that is,
>>> and hence of lot of things to be got wrong.
>>>
>>> And then there is the problem of calling code being in rings 1 or 2:
>>> Page tables don't guard ring 0 against such, and we don't even have
>>> the notion of selectors (and hence address ranges) bounding
>>> accessible regions. We can't even say we assume all of them to be
>>> %ds-relative, as it would certainly be legitimate for such a structure
>>> to e.g. live on the stack. Of course an option would be to require
>>> the kernel driver to not allow requests from other than ring 3.
>>>
>>> Plus finally - how would we tell interface structures coming from a
>>> kernel invoked hypercall from those originating from user mode?
>>> There would need to be at least some kind of flag then, which the
>>> privcmd driver set, but normal hypercalls originating in the kernel
>>> don't. Or would you envision to allow this DMOP hypercall to only
>>> be made by user mode tools? If so, does stubdom run its qemu in
>>> ring 3 or rather in ring 0?
>>>
>>> [breaking the order of quoting]
 And unless we're positive the guest kernel will never need these
 hypercalls, we probably need a flag that allows kernel-mode pointers.
>>>
>>> Ah, you actually mention that already.
>>>
>>  (Would it be sufficient to check the starts, or would
>> the ends need to be checked too?)
>
> Both would need to be checked, so the size field(s) would need to
> be locatable too.

 We could have the "fixed" part of the structure contain domid and an
 array of  which the privcmd driver could check.  I don't think
 that would be terrible.
>>>
>>> Doable, yes, but not really nice, especially for the party invoking
>>> the hypercall as well as the backing implementation in Xen (as
>>> opposed to the privcmd driver, for which such a model would likely
>>> work quite well), as they  then can't use the normal, simple reading
>>> of structure fields, but instead would need to populate array
>>> elements in the right order.
>>
>> So on the whole, what would be your suggestion for how to solve the
>> userspace-pointer problem?
> 
> Well, none of the options considered so far are really nice or
> readily available. I think the easiest to use for both the caller and
> the implementation of the hypercall would be the auxiliary
> hypercall for a kernel to indicate user/kernel boundaries plus a
> flag on the DMOP one for the kernel mode driver to indicate its
> user mode origin. The main (purely theoretical afaict) downside
> of this is the difficulty to use it in OSes with variable user/kernel
> boundaries.

What about including in the "fixed" part of the hypercall a virtual
address range that all pointers must be in?  That wouldn't even require
a user/kernel flag actually; and could conceivably be used by the caller
(either userspace or kernel space) to thwart certain kinds of potential
attacks.

It would take changing the copy_guest() macros to include a potential
range argument, but that shouldn't be too intrusive on the whole, I
wouldn't think.

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 15.08.16 at 11:39,  wrote:
> On 12/08/16 12:50, Jan Beulich wrote:
> On 12.08.16 at 11:44,  wrote:
>>> On 09/08/16 12:30, Jan Beulich wrote:
>>> On 09.08.16 at 12:48,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> Actually, having thought about this some more, and taking this
>> together with the expectations to the privcmd driver previously
>> outlined, I think this part is problematic: If all the driver is to know
>> is the position (within the interface structure) of the target domain
>> ID, then any guest handles embedded in the interface structure
>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>> validated, and hence user mode code would have a way to access
>> or modify kernel memory.
>
> Could the hypervisor know the difference between user and kernel
> memory, in principle ?

 Not without further new hypercalls, as the guest kernel would need
 to tell Xen what address ranges are kernel vs user (and that implies
 that any OS wishing to be able to act as Dom0 has a uniform
 separation of address spaces).
>>>
>>> Couldn't Xen tell from the guest pagetables whether the memory being
>>> accessed was user-mode or kernel mode?
>> 
>> That would be possible, but would feel like adding heuristics instead
>> of a proper distinction. Clearly we'd already be in some trouble if
>> there were cases where some structure doesn't get written to (and
>> hence could live in user-r/o mapped space), but others would need
>> to be verified to be user-r/w mapped. A lot of special casing, that is,
>> and hence of lot of things to be got wrong.
>> 
>> And then there is the problem of calling code being in rings 1 or 2:
>> Page tables don't guard ring 0 against such, and we don't even have
>> the notion of selectors (and hence address ranges) bounding
>> accessible regions. We can't even say we assume all of them to be
>> %ds-relative, as it would certainly be legitimate for such a structure
>> to e.g. live on the stack. Of course an option would be to require
>> the kernel driver to not allow requests from other than ring 3.
>> 
>> Plus finally - how would we tell interface structures coming from a
>> kernel invoked hypercall from those originating from user mode?
>> There would need to be at least some kind of flag then, which the
>> privcmd driver set, but normal hypercalls originating in the kernel
>> don't. Or would you envision to allow this DMOP hypercall to only
>> be made by user mode tools? If so, does stubdom run its qemu in
>> ring 3 or rather in ring 0?
>> 
>> [breaking the order of quoting]
>>> And unless we're positive the guest kernel will never need these
>>> hypercalls, we probably need a flag that allows kernel-mode pointers.
>> 
>> Ah, you actually mention that already.
>> 
>  (Would it be sufficient to check the starts, or would
> the ends need to be checked too?)

 Both would need to be checked, so the size field(s) would need to
 be locatable too.
>>>
>>> We could have the "fixed" part of the structure contain domid and an
>>> array of  which the privcmd driver could check.  I don't think
>>> that would be terrible.
>> 
>> Doable, yes, but not really nice, especially for the party invoking
>> the hypercall as well as the backing implementation in Xen (as
>> opposed to the privcmd driver, for which such a model would likely
>> work quite well), as they  then can't use the normal, simple reading
>> of structure fields, but instead would need to populate array
>> elements in the right order.
> 
> So on the whole, what would be your suggestion for how to solve the
> userspace-pointer problem?

Well, none of the options considered so far are really nice or
readily available. I think the easiest to use for both the caller and
the implementation of the hypercall would be the auxiliary
hypercall for a kernel to indicate user/kernel boundaries plus a
flag on the DMOP one for the kernel mode driver to indicate its
user mode origin. The main (purely theoretical afaict) downside
of this is the difficulty to use it in OSes with variable user/kernel
boundaries.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 12/08/16 12:50, Jan Beulich wrote:
 On 12.08.16 at 11:44,  wrote:
>> On 09/08/16 12:30, Jan Beulich wrote:
>> On 09.08.16 at 12:48,  wrote:
 Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
 depriv)"):
> Actually, having thought about this some more, and taking this
> together with the expectations to the privcmd driver previously
> outlined, I think this part is problematic: If all the driver is to know
> is the position (within the interface structure) of the target domain
> ID, then any guest handles embedded in the interface structure
> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
> validated, and hence user mode code would have a way to access
> or modify kernel memory.

 Could the hypervisor know the difference between user and kernel
 memory, in principle ?
>>>
>>> Not without further new hypercalls, as the guest kernel would need
>>> to tell Xen what address ranges are kernel vs user (and that implies
>>> that any OS wishing to be able to act as Dom0 has a uniform
>>> separation of address spaces).
>>
>> Couldn't Xen tell from the guest pagetables whether the memory being
>> accessed was user-mode or kernel mode?
> 
> That would be possible, but would feel like adding heuristics instead
> of a proper distinction. Clearly we'd already be in some trouble if
> there were cases where some structure doesn't get written to (and
> hence could live in user-r/o mapped space), but others would need
> to be verified to be user-r/w mapped. A lot of special casing, that is,
> and hence of lot of things to be got wrong.
> 
> And then there is the problem of calling code being in rings 1 or 2:
> Page tables don't guard ring 0 against such, and we don't even have
> the notion of selectors (and hence address ranges) bounding
> accessible regions. We can't even say we assume all of them to be
> %ds-relative, as it would certainly be legitimate for such a structure
> to e.g. live on the stack. Of course an option would be to require
> the kernel driver to not allow requests from other than ring 3.
> 
> Plus finally - how would we tell interface structures coming from a
> kernel invoked hypercall from those originating from user mode?
> There would need to be at least some kind of flag then, which the
> privcmd driver set, but normal hypercalls originating in the kernel
> don't. Or would you envision to allow this DMOP hypercall to only
> be made by user mode tools? If so, does stubdom run its qemu in
> ring 3 or rather in ring 0?
> 
> [breaking the order of quoting]
>> And unless we're positive the guest kernel will never need these
>> hypercalls, we probably need a flag that allows kernel-mode pointers.
> 
> Ah, you actually mention that already.
> 
  (Would it be sufficient to check the starts, or would
 the ends need to be checked too?)
>>>
>>> Both would need to be checked, so the size field(s) would need to
>>> be locatable too.
>>
>> We could have the "fixed" part of the structure contain domid and an
>> array of  which the privcmd driver could check.  I don't think
>> that would be terrible.
> 
> Doable, yes, but not really nice, especially for the party invoking
> the hypercall as well as the backing implementation in Xen (as
> opposed to the privcmd driver, for which such a model would likely
> work quite well), as they  then can't use the normal, simple reading
> of structure fields, but instead would need to populate array
> elements in the right order.

So on the whole, what would be your suggestion for how to solve the
userspace-pointer problem?

 -George


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 12.08.16 at 11:44,  wrote:
> On 09/08/16 12:30, Jan Beulich wrote:
> On 09.08.16 at 12:48,  wrote:
>>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>>> depriv)"):
 Actually, having thought about this some more, and taking this
 together with the expectations to the privcmd driver previously
 outlined, I think this part is problematic: If all the driver is to know
 is the position (within the interface structure) of the target domain
 ID, then any guest handles embedded in the interface structure
 (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
 validated, and hence user mode code would have a way to access
 or modify kernel memory.
>>>
>>> Could the hypervisor know the difference between user and kernel
>>> memory, in principle ?
>> 
>> Not without further new hypercalls, as the guest kernel would need
>> to tell Xen what address ranges are kernel vs user (and that implies
>> that any OS wishing to be able to act as Dom0 has a uniform
>> separation of address spaces).
> 
> Couldn't Xen tell from the guest pagetables whether the memory being
> accessed was user-mode or kernel mode?

That would be possible, but would feel like adding heuristics instead
of a proper distinction. Clearly we'd already be in some trouble if
there were cases where some structure doesn't get written to (and
hence could live in user-r/o mapped space), but others would need
to be verified to be user-r/w mapped. A lot of special casing, that is,
and hence of lot of things to be got wrong.

And then there is the problem of calling code being in rings 1 or 2:
Page tables don't guard ring 0 against such, and we don't even have
the notion of selectors (and hence address ranges) bounding
accessible regions. We can't even say we assume all of them to be
%ds-relative, as it would certainly be legitimate for such a structure
to e.g. live on the stack. Of course an option would be to require
the kernel driver to not allow requests from other than ring 3.

Plus finally - how would we tell interface structures coming from a
kernel invoked hypercall from those originating from user mode?
There would need to be at least some kind of flag then, which the
privcmd driver set, but normal hypercalls originating in the kernel
don't. Or would you envision to allow this DMOP hypercall to only
be made by user mode tools? If so, does stubdom run its qemu in
ring 3 or rather in ring 0?

[breaking the order of quoting]
> And unless we're positive the guest kernel will never need these
> hypercalls, we probably need a flag that allows kernel-mode pointers.

Ah, you actually mention that already.

>>>  (Would it be sufficient to check the starts, or would
>>> the ends need to be checked too?)
>> 
>> Both would need to be checked, so the size field(s) would need to
>> be locatable too.
>
> We could have the "fixed" part of the structure contain domid and an
> array of  which the privcmd driver could check.  I don't think
> that would be terrible.

Doable, yes, but not really nice, especially for the party invoking
the hypercall as well as the backing implementation in Xen (as
opposed to the privcmd driver, for which such a model would likely
work quite well), as they  then can't use the normal, simple reading
of structure fields, but instead would need to populate array
elements in the right order.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 09/08/16 12:30, Jan Beulich wrote:
 On 09.08.16 at 12:48,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> Actually, having thought about this some more, and taking this
>>> together with the expectations to the privcmd driver previously
>>> outlined, I think this part is problematic: If all the driver is to know
>>> is the position (within the interface structure) of the target domain
>>> ID, then any guest handles embedded in the interface structure
>>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>>> validated, and hence user mode code would have a way to access
>>> or modify kernel memory.
>>
>> Could the hypervisor know the difference between user and kernel
>> memory, in principle ?
> 
> Not without further new hypercalls, as the guest kernel would need
> to tell Xen what address ranges are kernel vs user (and that implies
> that any OS wishing to be able to act as Dom0 has a uniform
> separation of address spaces).

Couldn't Xen tell from the guest pagetables whether the memory being
accessed was user-mode or kernel mode?

>> Alternatively, would it be possible for the ABI to specify somehow
>> what parameters are guest handles, so that the privcmd driver could
>> check them ?
> 
> We could presumably invent something, but I'm afraid it would end
> up quite ugly.
> 
>>  (Would it be sufficient to check the starts, or would
>> the ends need to be checked too?)
> 
> Both would need to be checked, so the size field(s) would need to
> be locatable too.

We could have the "fixed" part of the structure contain domid and an
array of  which the privcmd driver could check.  I don't think
that would be terrible.

Alternately, the "fixed" part of the hypercall could contain a
 range, which if non-zero, Xen should use to check any
pointer contained in the struct -- that would be more flexible probably.

Or we could do as Jan hints at above -- have some way to have dom0
communicate the kernel address range to Xen (either via hypercall, or
maybe via the shared info page) so that Xen just knows the address
layout for any individual domain.

And unless we're positive the guest kernel will never need these
hypercalls, we probably need a flag that allows kernel-mode pointers.

It's worth pointing out that the problem Xen has distinguishing
user/kernel mode pointers is the same even if we use the alternate
suggestion of per-process XSM labels.

 -George

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 09.08.16 at 12:48,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> Actually, having thought about this some more, and taking this
>> together with the expectations to the privcmd driver previously
>> outlined, I think this part is problematic: If all the driver is to know
>> is the position (within the interface structure) of the target domain
>> ID, then any guest handles embedded in the interface structure
>> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
>> validated, and hence user mode code would have a way to access
>> or modify kernel memory.
> 
> Could the hypervisor know the difference between user and kernel
> memory, in principle ?

Not without further new hypercalls, as the guest kernel would need
to tell Xen what address ranges are kernel vs user (and that implies
that any OS wishing to be able to act as Dom0 has a uniform
separation of address spaces).

> Alternatively, would it be possible for the ABI to specify somehow
> what parameters are guest handles, so that the privcmd driver could
> check them ?

We could presumably invent something, but I'm afraid it would end
up quite ugly.

>  (Would it be sufficient to check the starts, or would
> the ends need to be checked too?)

Both would need to be checked, so the size field(s) would need to
be locatable too.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> Actually, having thought about this some more, and taking this
> together with the expectations to the privcmd driver previously
> outlined, I think this part is problematic: If all the driver is to know
> is the position (within the interface structure) of the target domain
> ID, then any guest handles embedded in the interface structure
> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
> validated, and hence user mode code would have a way to access
> or modify kernel memory.

Could the hypervisor know the difference between user and kernel
memory, in principle ?

Alternatively, would it be possible for the ABI to specify somehow
what parameters are guest handles, so that the privcmd driver could
check them ?  (Would it be sufficient to check the starts, or would
the ends need to be checked too?)

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 08.08.16 at 15:46,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 05.08.16 at 18:28,  wrote:
>> > That is, a bug of class 2 would allow the unprivileged qemu process in
>> > dom0 to cause damage to other parts of dom0.
> ...
>> Ah, okay, I think I finally understand. [...]
>> 
>> I'm having, however, a hard time imagining a class 2 bug for any
>> of the hvmop-s that are being converted by the hvmctl series:
>> These act on the target domain, so would not touch the calling
>> ones state other than for copying argument structures to/from
>> guest memory (and I don't view mixing up domain pointers as
>> a likely source of problems).
> 
> Right.  AIUI all the hypercall arguments are passed using
> calling-guest-virtual addresses, which the dom0 privcmd arrangements
> are capable of ensuring are sane.

Actually, having thought about this some more, and taking this
together with the expectations to the privcmd driver previously
outlined, I think this part is problematic: If all the driver is to know
is the position (within the interface structure) of the target domain
ID, then any guest handles embedded in the interface structure
(XEN_HVMCTL_track_dirty_vram only for now) couldn't get
validated, and hence user mode code would have a way to access
or modify kernel memory.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 08.08.16 at 15:46,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> I'm having, however, a hard time imagining a class 2 bug for any
>> of the hvmop-s that are being converted by the hvmctl series:
>> These act on the target domain, so would not touch the calling
>> ones state other than for copying argument structures to/from
>> guest memory (and I don't view mixing up domain pointers as
>> a likely source of problems).
> 
> Right.  AIUI all the hypercall arguments are passed using
> calling-guest-virtual addresses, which the dom0 privcmd arrangements
> are capable of ensuring are sane.
> 
>> Any other problem they might
>> reasonably have would then affect the system as a whole (class
>> 1) or just the target domain (non-security bug).
> 
> So would it therefore be OK to introduce the enhanced security promise
> - the lack of `class 2' bugs - for HVMCTL from the beginning ?

I think so, since ...

> This would involve a small amount of extra thought for each invididual
> hypercall, just to check that the assumptions we are relying on (as
> you put them above) are not violated.

... this looks to be a manageable amount of code auditing (albeit
I'd like to see whether someone else can perhaps come up with
some potential, more realistic kind of bug that could fall into class
2 before volunteering to make an attempt at doing such an audit).

> If so then good, but I think we still need to have a proper
> conversation about how we are going to provide ABI stability to qemu
> in practice.

Yes, the ABI stability aspect still needs coming to an agreement.
Since relaxation is easier than tightening, I'm going to retain the
current way the patches are in this regard, i.e. assume an
unstable hypercall ABI.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 05.08.16 at 18:28,  wrote:
> > That is, a bug of class 2 would allow the unprivileged qemu process in
> > dom0 to cause damage to other parts of dom0.
...
> Ah, okay, I think I finally understand. [...]
> 
> I'm having, however, a hard time imagining a class 2 bug for any
> of the hvmop-s that are being converted by the hvmctl series:
> These act on the target domain, so would not touch the calling
> ones state other than for copying argument structures to/from
> guest memory (and I don't view mixing up domain pointers as
> a likely source of problems).

Right.  AIUI all the hypercall arguments are passed using
calling-guest-virtual addresses, which the dom0 privcmd arrangements
are capable of ensuring are sane.

> Any other problem they might
> reasonably have would then affect the system as a whole (class
> 1) or just the target domain (non-security bug).

So would it therefore be OK to introduce the enhanced security promise
- the lack of `class 2' bugs - for HVMCTL from the beginning ?

This would involve a small amount of extra thought for each invididual
hypercall, just to check that the assumptions we are relying on (as
you put them above) are not violated.

If so then good, but I think we still need to have a proper
conversation about how we are going to provide ABI stability to qemu
in practice.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 05.08.16 at 18:28,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 04.08.16 at 13:21,  wrote:
>> > What we cannot do is audit every HVMCTL, fix the class 2 problems, and
>> > then declare HVMCTL to have the relevant security property, and
>> > implement corresponding code in dom0's privcmd drivers which relies on
>> > the security property.  This is because the dom0 privcmd driver
>> > doesn't know whether the HVMCTLs it is allowing not-fully-trusted
>> > userspace to make are actually trustworthy (with the specific
>> > hypervisor version in question.)
>> 
>> I continue to not really understand this argumentation: Dom0's
>> privcmd driver doesn't really matter here. If there's a bug in
>> something qemu uses, this is a problem no matter whether that
>> operation gets called though the to-be-added privcmd logic, or
>> straight from a stubdom qemu. Both are less than fully privileged.
>> What do I continue to be missing?
> 
> Let me try again.  Earlier I wrote:
> 
>   AFAICT there are two kinds of possible bug:
> 
>   1. An HVMCTL (or hvmop) that can have an adverse affect on the whole
>   system.  Such bugs would already be security bugs, covered by our
>   security support policy.  Such a bug would be a security bug whether
>   the operation is an hvmop or an HVMCTL or a DMOP.
> 
>   2. An HVMCTL (or hvmop) that can have an adverse effect on the calling
>   domain.  Such bugs are not currently security bugs.  But the of qemu
>   depriv project requires them to be removed.  Such such a bug is a
>   security bug if it is a DMOP[1] but not otherwise.
> 
> Bugs of class 1 are already security bugs.  They can already be
> exploited by stub device models.
> 
> Bugs of class 2 are only security bugs if we allow unprivileged
> callers within a privileged domain to use the corresponding hypercall.
> 
> That is, a bug of class 2 would allow the unprivileged qemu process in
> dom0 to cause damage to other parts of dom0.
> 
> Bugs of class 2 are of no interest to an attacker who has control of a
> stub device model, because all it allows them to do is damage the
> device domain domain (which they already control).

Ah, okay, I think I finally understand. I'm sorry for being dense.

I'm having, however, a hard time imagining a class 2 bug for any
of the hvmop-s that are being converted by the hvmctl series:
These act on the target domain, so would not touch the calling
ones state other than for copying argument structures to/from
guest memory (and I don't view mixing up domain pointers as
a likely source of problems). Any other problem they might
reasonably have would then affect the system as a whole (class
1) or just the target domain (non-security bug).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 04.08.16 at 13:21,  wrote:
> > What we cannot do is audit every HVMCTL, fix the class 2 problems, and
> > then declare HVMCTL to have the relevant security property, and
> > implement corresponding code in dom0's privcmd drivers which relies on
> > the security property.  This is because the dom0 privcmd driver
> > doesn't know whether the HVMCTLs it is allowing not-fully-trusted
> > userspace to make are actually trustworthy (with the specific
> > hypervisor version in question.)
> 
> I continue to not really understand this argumentation: Dom0's
> privcmd driver doesn't really matter here. If there's a bug in
> something qemu uses, this is a problem no matter whether that
> operation gets called though the to-be-added privcmd logic, or
> straight from a stubdom qemu. Both are less than fully privileged.
> What do I continue to be missing?

Let me try again.  Earlier I wrote:

  AFAICT there are two kinds of possible bug:

  1. An HVMCTL (or hvmop) that can have an adverse affect on the whole
  system.  Such bugs would already be security bugs, covered by our
  security support policy.  Such a bug would be a security bug whether
  the operation is an hvmop or an HVMCTL or a DMOP.

  2. An HVMCTL (or hvmop) that can have an adverse effect on the calling
  domain.  Such bugs are not currently security bugs.  But the of qemu
  depriv project requires them to be removed.  Such such a bug is a
  security bug if it is a DMOP[1] but not otherwise.

Bugs of class 1 are already security bugs.  They can already be
exploited by stub device models.

Bugs of class 2 are only security bugs if we allow unprivileged
callers within a privileged domain to use the corresponding hypercall.

That is, a bug of class 2 would allow the unprivileged qemu process in
dom0 to cause damage to other parts of dom0.

Bugs of class 2 are of no interest to an attacker who has control of a
stub device model, because all it allows them to do is damage the
device domain domain (which they already control).

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 04.08.16 at 13:21,  wrote:
> What we cannot do is audit every HVMCTL, fix the class 2 problems, and
> then declare HVMCTL to have the relevant security property, and
> implement corresponding code in dom0's privcmd drivers which relies on
> the security property.  This is because the dom0 privcmd driver
> doesn't know whether the HVMCTLs it is allowing not-fully-trusted
> userspace to make are actually trustworthy (with the specific
> hypervisor version in question.)

I continue to not really understand this argumentation: Dom0's
privcmd driver doesn't really matter here. If there's a bug in
something qemu uses, this is a problem no matter whether that
operation gets called though the to-be-added privcmd logic, or
straight from a stubdom qemu. Both are less than fully privileged.
What do I continue to be missing?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 03.08.16 at 18:10,  wrote:
> > George Dunlap writes ("Re: Device model operation hypercall (DMOP, re qemu 
> > depriv)"):
> >> So before qemu devpriv can be usable, *all* the HVMCTL operations would
> >> need to be audited, and those that were deemed insecure would need to be
> >> either fixed or removed.
> > 
> > Even worse, the bad HVMCTLs would be retrospectively turned into
> > security-bugs-in-old-hypervisors.  I don't think this is tenable.
> 
> How would a bug in the respective current hvmop then not be a
> security issue as well?

AFAICT there are two kinds of possible bug:

1. An HVMCTL (or hvmop) that can have an adverse affect on the whole
system.  Such bugs would already be security bugs, covered by our
security support policy.  Such a bug would be a security bug whether
the operation is an hvmop or an HVMCTL or a DMOP.

2. An HVMCTL (or hvmop) that can have an adverse effect on the calling
domain.  Such bugs are not currently security bugs.  But the of qemu
depriv project requires them to be removed.  Such such a bug is a
security bug if it is a DMOP[1] but not otherwise.

By "is a security bug if it is a DMOP" I mean "is a security bug if
exposed via a hypercall which promises the security property necessary
for DMOP, even if that hypercall is called something else".

Strictly speaking, in order to move existing hypercalls into a new
DMOP hypercall, no further auditing is required for bugs of class 1
(other than that the privilege check is correct, ie that the new DMOP
is indeed currently available to stub device models).

But in order to move hypercalls into a new DMOP hypercall, they need
to be checked for bugs of class 2.

If we move some hypercalls which have potential bugs of class 2 from
hvmop to HVMCTL, and then later think we want to create something like
DMOP, we have a number of choices:

We can invent a new DMOP hypercall and start moving everything from
HVMCTL to DMOP as we audit it.

We could audit every HVMCTL, recording the audit status in-tree
somewhere, moving hypercalls with class 2 problems out of HVMCTL to a
new hypercall, and eventually, after we have audited everything, we
could change HVMCTL's hypercall number (and maybe rename it DMOP).

What we cannot do is audit every HVMCTL, fix the class 2 problems, and
then declare HVMCTL to have the relevant security property, and
implement corresponding code in dom0's privcmd drivers which relies on
the security property.  This is because the dom0 privcmd driver
doesn't know whether the HVMCTLs it is allowing not-fully-trusted
userspace to make are actually trustworthy (with the specific
hypervisor version in question.)

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 03.08.16 at 18:10,  wrote:
> George Dunlap writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> So before qemu devpriv can be usable, *all* the HVMCTL operations would
>> need to be audited, and those that were deemed insecure would need to be
>> either fixed or removed.
> 
> Even worse, the bad HVMCTLs would be retrospectively turned into
> security-bugs-in-old-hypervisors.  I don't think this is tenable.

How would a bug in the respective current hvmop then not be a
security issue as well?

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

George Dunlap writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> So before qemu devpriv can be usable, *all* the HVMCTL operations would
> need to be audited, and those that were deemed insecure would need to be
> either fixed or removed.

Even worse, the bad HVMCTLs would be retrospectively turned into
security-bugs-in-old-hypervisors.  I don't think this is tenable.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 03.08.16 at 15:37,  wrote:
> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On 03.08.16 at 12:29,  wrote:
>> > Does that mean that functionality exposed by all the prooposed HVMCTLs
>> > is currently available to stubdoms ?
>> 
>> That series only moves code from one hypercall to another (new) one,
>> without any security implications at all. What has been available to
>> stubdoms prior to that series will be available the same way once it
>> got applied.
> 
> So what you mean is that in HVMCTL, the privilege check is retained in
> the individual HVMCTL sub-operation ?  (Ie what used to be IS_PRIV or
> IS_PRIV_FOR - and implicitly, some sub-ops would be IS_PRIV and some
> IS_PRIV_FOR.)
> 
> But looking at your v2 01/11, I see this in do_hvmctl:
> 
>+rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
>+if ( rc )
>+{
>+rcu_unlock_domain(d);
>+return rc;
>+}
> 
> And looking at v2 02/11, I didn't see any privilege check in the
> specific hypercall.

Of course - there were no explicit IS_PRIV{,_FOR}() uses before
(i.e. the patch also doesn't remove any), these all sit behind the
XSM hook. And no, there are no per-sub-op privilege checks,
they're being consolidated to just the one you quote above.

> With DMOP it would make sense for the privilege check to be
> IS_PRIV_FOR, in the DMOP dispatcher.  But it seems that this privilege
> check already exists in HVMCTL in the right form.
> 
> So AFAICT HVMCTLs already guarantee not to have an adverse impact on
> the whole system.  If that weren't the case, then a stub dm could
> exploit the situation.

And to tell you the truth, I'm not entirely convinced that all the
code implementing those operations (be it in there current hvmop
form or the new hvmctl one - again, the series only moves code
around) is really safe in this regard. But with there even being
at least one domctl not on xsm-flask.txt's safe-for-disaggregation
list, but reportedly used by qemu (I don't recall right now which
exact one it is), stubdom-s can't be considered fully secure right
now anyway.

> Is the audit that is required, to check that the DMOP doesn't have an
> adverse effect on the _calling domain_ ?  AFAICT most HVMCTLs/DMOPs
> have /no/ effect on the calling domain, other than as part of argument
> passing.  So that audit should be easy.
> 
> So I am confused.  What am I missing ?

The main adverse effect on the whole system that I can see
would be long latency operations, but I can't exclude others: Just
look at the final patch of the series, which fixes a serialization bug
which I noticed while doing the code movement. I don't think lack
of proper serialization is guaranteed to only affect the target
domain.

>> > Now, there may be other ways to represent/record the security status.
>> > But it will be necessary to either (i) avoid violating the DMOP
>> > security promise, by making questionable calls not available via DMOP
>> > or (ii) trying to retrofit the security promise to DMOP later.
>> > 
>> > I think (ii) is not a good approach.  It would amount to introducing a
>> > whole new set of interfaces, and then later trying to redefine them to
>> > have a particular security property which was not originally there.
>> 
>> I agree that (i) would be the better approach, but I don't think I
>> can assess when I would find the time to do the required auditing
>> of all involved code. Plus I don't see the difference between going
>> the (ii) route with the presented hvmctl series vs keeping things as
>> they are (under hvmop) - in both cases the security promise will
>> need to be retrofit onto existing code.
> 
> If we don't apply HVMCTL, we can introduce DMOP and then individually
> move hypercalls from hvmop to DMOP as they are audited.
> 
> Would a similar approach be acceptable even after HVMCTL ?
> 
> That is, the following plan:
> 
> 1. Apply HVMCTL right away.  This solves the cleanup problem,
>but leaves the qemu depriv problem unsolved.
> 
> 2. After the necessary discussion to understand and refine the DMOP
>design, create a new DMOP hypercall with appropriate security
>promises and whatever stability promises are agreed, but with no
>sub-ops.
> 
> 3. Move each HVMCTL to DMOP, one by one, as it is audited.
> 
> 4. Perhaps some HVMCTLs will remain which are inherently unsuitable
>for use with qemu depriv.  If not, perhaps eventually delete HVMCTL
>entirely, or leave it empty (with no subops).
> 
> This has the downside that we end up moving all the DMOPs twice.  But
> it does allow us to separate the audit work from the cleanup/reorg.

Well, moving things twice doesn't sound attractive. I was rather
thinking of declaring hvmctl (or dmop, if that's to intended name)
sub-ops secure one by one as they get audited. Part of my reason
to be hesitant to do such an audit myself (apart from the time

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 03/08/16 15:16, Jan Beulich wrote:
 On 03.08.16 at 15:37,  wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> On 03.08.16 at 12:29,  wrote:
 Does that mean that functionality exposed by all the prooposed HVMCTLs
 is currently available to stubdoms ?
>>>
>>> That series only moves code from one hypercall to another (new) one,
>>> without any security implications at all. What has been available to
>>> stubdoms prior to that series will be available the same way once it
>>> got applied.
>>
>> So what you mean is that in HVMCTL, the privilege check is retained in
>> the individual HVMCTL sub-operation ?  (Ie what used to be IS_PRIV or
>> IS_PRIV_FOR - and implicitly, some sub-ops would be IS_PRIV and some
>> IS_PRIV_FOR.)
>>
>> But looking at your v2 01/11, I see this in do_hvmctl:
>>
>>+rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
>>+if ( rc )
>>+{
>>+rcu_unlock_domain(d);
>>+return rc;
>>+}
>>
>> And looking at v2 02/11, I didn't see any privilege check in the
>> specific hypercall.
> 
> Of course - there were no explicit IS_PRIV{,_FOR}() uses before
> (i.e. the patch also doesn't remove any), these all sit behind the
> XSM hook. And no, there are no per-sub-op privilege checks,
> they're being consolidated to just the one you quote above.
> 
>> With DMOP it would make sense for the privilege check to be
>> IS_PRIV_FOR, in the DMOP dispatcher.  But it seems that this privilege
>> check already exists in HVMCTL in the right form.
>>
>> So AFAICT HVMCTLs already guarantee not to have an adverse impact on
>> the whole system.  If that weren't the case, then a stub dm could
>> exploit the situation.
> 
> And to tell you the truth, I'm not entirely convinced that all the
> code implementing those operations (be it in there current hvmop
> form or the new hvmctl one - again, the series only moves code
> around) is really safe in this regard. But with there even being
> at least one domctl not on xsm-flask.txt's safe-for-disaggregation
> list, but reportedly used by qemu (I don't recall right now which
> exact one it is), stubdom-s can't be considered fully secure right
> now anyway.
> 
>> Is the audit that is required, to check that the DMOP doesn't have an
>> adverse effect on the _calling domain_ ?  AFAICT most HVMCTLs/DMOPs
>> have /no/ effect on the calling domain, other than as part of argument
>> passing.  So that audit should be easy.
>>
>> So I am confused.  What am I missing ?
> 
> The main adverse effect on the whole system that I can see
> would be long latency operations, but I can't exclude others: Just
> look at the final patch of the series, which fixes a serialization bug
> which I noticed while doing the code movement. I don't think lack
> of proper serialization is guaranteed to only affect the target
> domain.
> 
 Now, there may be other ways to represent/record the security status.
 But it will be necessary to either (i) avoid violating the DMOP
 security promise, by making questionable calls not available via DMOP
 or (ii) trying to retrofit the security promise to DMOP later.

 I think (ii) is not a good approach.  It would amount to introducing a
 whole new set of interfaces, and then later trying to redefine them to
 have a particular security property which was not originally there.
>>>
>>> I agree that (i) would be the better approach, but I don't think I
>>> can assess when I would find the time to do the required auditing
>>> of all involved code. Plus I don't see the difference between going
>>> the (ii) route with the presented hvmctl series vs keeping things as
>>> they are (under hvmop) - in both cases the security promise will
>>> need to be retrofit onto existing code.
>>
>> If we don't apply HVMCTL, we can introduce DMOP and then individually
>> move hypercalls from hvmop to DMOP as they are audited.
>>
>> Would a similar approach be acceptable even after HVMCTL ?
>>
>> That is, the following plan:
>>
>> 1. Apply HVMCTL right away.  This solves the cleanup problem,
>>but leaves the qemu depriv problem unsolved.
>>
>> 2. After the necessary discussion to understand and refine the DMOP
>>design, create a new DMOP hypercall with appropriate security
>>promises and whatever stability promises are agreed, but with no
>>sub-ops.
>>
>> 3. Move each HVMCTL to DMOP, one by one, as it is audited.
>>
>> 4. Perhaps some HVMCTLs will remain which are inherently unsuitable
>>for use with qemu depriv.  If not, perhaps eventually delete HVMCTL
>>entirely, or leave it empty (with no subops).
>>
>> This has the downside that we end up moving all the DMOPs twice.  But
>> it does allow us to separate the audit work from the cleanup/reorg.
> 
> Well, moving things twice doesn't sound attractive. I was rather
> thinking of declaring hvmctl (or dmop, if that's to intended 

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
depriv)"):
> On 03.08.16 at 12:29,  wrote:
> > I thought it useful to set out the DMOP proposal from first
> > principles, with clear motivation, discussion of not-chosen
> > alternatives, and of course with a clear statement of the principles
> > of operation and of the security design.
> 
> Okay; nevertheless I did get the feeling that some of this was
> prompted by the hvmctl series posting.

Well, I think the HVMCTL series is an important part of this
conversation - even though this DMOP idea is motivated by the qemu
depriv proposals (initiated mostly by Stefano in the last release
cycle).

> > Does that mean that functionality exposed by all the prooposed HVMCTLs
> > is currently available to stubdoms ?
> 
> That series only moves code from one hypercall to another (new) one,
> without any security implications at all. What has been available to
> stubdoms prior to that series will be available the same way once it
> got applied.

So what you mean is that in HVMCTL, the privilege check is retained in
the individual HVMCTL sub-operation ?  (Ie what used to be IS_PRIV or
IS_PRIV_FOR - and implicitly, some sub-ops would be IS_PRIV and some
IS_PRIV_FOR.)

But looking at your v2 01/11, I see this in do_hvmctl:

   +rc = xsm_hvm_control(XSM_DM_PRIV, d, op.cmd);
   +if ( rc )
   +{
   +rcu_unlock_domain(d);
   +return rc;
   +}

And looking at v2 02/11, I didn't see any privilege check in the
specific hypercall.

With DMOP it would make sense for the privilege check to be
IS_PRIV_FOR, in the DMOP dispatcher.  But it seems that this privilege
check already exists in HVMCTL in the right form.

So AFAICT HVMCTLs already guarantee not to have an adverse impact on
the whole system.  If that weren't the case, then a stub dm could
exploit the situation.

Is the audit that is required, to check that the DMOP doesn't have an
adverse effect on the _calling domain_ ?  AFAICT most HVMCTLs/DMOPs
have /no/ effect on the calling domain, other than as part of argument
passing.  So that audit should be easy.

So I am confused.  What am I missing ?

> > If the 01/ patch contains such promises, then logically the 02/ patch
> > which introduces the first DMOP is extending that promise to that
> > operation.  It is at that point that the security decision should be
> > made.
> 
> Correct. Yet again the original goal of the series was just proper
> separation of two groups of operations that should never have
> been all thrown under the same hypercall.

What I am doing is presenting another, additional goal.  I don't
dispute the usefulness of the HVMCTL cleanup.  But I want to
understand our future intentions.

In particular, as a proponent of the DMOP idea, I want to avoid the
situation that my idea is blocked somehow by a conflicting series.

> > Now, there may be other ways to represent/record the security status.
> > But it will be necessary to either (i) avoid violating the DMOP
> > security promise, by making questionable calls not available via DMOP
> > or (ii) trying to retrofit the security promise to DMOP later.
> > 
> > I think (ii) is not a good approach.  It would amount to introducing a
> > whole new set of interfaces, and then later trying to redefine them to
> > have a particular security property which was not originally there.
> 
> I agree that (i) would be the better approach, but I don't think I
> can assess when I would find the time to do the required auditing
> of all involved code. Plus I don't see the difference between going
> the (ii) route with the presented hvmctl series vs keeping things as
> they are (under hvmop) - in both cases the security promise will
> need to be retrofit onto existing code.

If we don't apply HVMCTL, we can introduce DMOP and then individually
move hypercalls from hvmop to DMOP as they are audited.

Would a similar approach be acceptable even after HVMCTL ?

That is, the following plan:

1. Apply HVMCTL right away.  This solves the cleanup problem,
   but leaves the qemu depriv problem unsolved.

2. After the necessary discussion to understand and refine the DMOP
   design, create a new DMOP hypercall with appropriate security
   promises and whatever stability promises are agreed, but with no
   sub-ops.

3. Move each HVMCTL to DMOP, one by one, as it is audited.

4. Perhaps some HVMCTLs will remain which are inherently unsuitable
   for use with qemu depriv.  If not, perhaps eventually delete HVMCTL
   entirely, or leave it empty (with no subops).

This has the downside that we end up moving all the DMOPs twice.  But
it does allow us to separate the audit work from the cleanup/reorg.

Regards,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 03.08.16 at 12:29,  wrote:
> Wei Liu writes ("Re: Device model operation hypercall (DMOP, re qemu 
> depriv)"):
>> On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
>> > > A DMOP is defined to never put at risk the stability or security of
>> > > the whole system, nor of the domain which calls DMOP.  However, a DMOP
>> > > may have arbitrary effects on the target domid.
>> > 
>> > With the exception of this and the privcmd layer described below,
>> > DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
>> > And the security aspect mentioned above won't disappear if we
>> > use DMOP instead of HVMCTL. So I don't see why the hvmctl
>> > series as is can't be the starting point of this, with the stability/
>> > security concerns addressed subsequently, for being orthogonal.
> 
> I don't (currently) have a clear understanding of how my proposed DMOP
> relates to HVMCTL.
> 
> I thought it useful to set out the DMOP proposal from first
> principles, with clear motivation, discussion of not-chosen
> alternatives, and of course with a clear statement of the principles
> of operation and of the security design.

Okay; nevertheless I did get the feeling that some of this was
prompted by the hvmctl series posting.

> The security property I have quoted above is absolutely critical to
> the DMOP proposal.  I'm a bit concerned by comments like the above
> `with the exception of this' (which seems to refer to the security
> property).

Indeed it does.

> Earlier during one of the HVMCTL threads I asked
> 
> This is a slight digression, but is it intended that all of these
> hvmctl's are safe to expose to a deprivileged device model process in
> dom0, or to a device model stub domain ?
> 
> Jan replied:
> 
> Yes, afaict (they've been exposed the same way before).
> 
> Does that mean that functionality exposed by all the prooposed HVMCTLs
> is currently available to stubdoms ?

That series only moves code from one hypercall to another (new) one,
without any security implications at all. What has been available to
stubdoms prior to that series will be available the same way once it
got applied.

>> >  So I don't see why the hvmctl series as is can't be the starting
>> > point of this, with the stability/ security concerns addressed
>> > subsequently, for being orthogonal.
> 
> Please don't misunderstand me as trying to compete with or block
> your HVMCTL work.  It may well be that HVMCTL is what I want, but:
> 
> If we adopt the design principles I describe in my DMOP proposal, I
> don't think the security concerns are separable.
> 
> ISTM that a patch series introducing DMOP should start with a patch
> which introduces the DMOP hypercall, with no sub-operations.
> 
> Such a patch would have code content very like that in
>   [PATCH 01/11] public / x86: introduce hvmctl hypercall
> 
> But, such a patch should also explain the semantics.  The Xen public
> headers ought to contain explanations of the promises that the
> hypervisor makes about DMOP.  Importantly:
>  - the promise that a DMOP cannot harm anyone except the target domid
>  - the ABI stability of the target domid field
>  - what the ABI stability policy is wrt the actual DMOPs themselves

Well, none of that was the original goal of the series; some of this
could be merged in.

> If the 01/ patch contains such promises, then logically the 02/ patch
> which introduces the first DMOP is extending that promise to that
> operation.  It is at that point that the security decision should be
> made.

Correct. Yet again the original goal of the series was just proper
separation of two groups of operations that should never have
been all thrown under the same hypercall.

> Now, there may be other ways to represent/record the security status.
> But it will be necessary to either (i) avoid violating the DMOP
> security promise, by making questionable calls not available via DMOP
> or (ii) trying to retrofit the security promise to DMOP later.
> 
> I think (ii) is not a good approach.  It would amount to introducing a
> whole new set of interfaces, and then later trying to redefine them to
> have a particular security property which was not originally there.

I agree that (i) would be the better approach, but I don't think I
can assess when I would find the time to do the required auditing
of all involved code. Plus I don't see the difference between going
the (ii) route with the presented hvmctl series vs keeping things as
they are (under hvmop) - in both cases the security promise will
need to be retrofit onto existing code.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Ian Jackson]

Wei Liu writes ("Re: Device model operation hypercall (DMOP, re qemu depriv)"):
> On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
> > > A DMOP is defined to never put at risk the stability or security of
> > > the whole system, nor of the domain which calls DMOP.  However, a DMOP
> > > may have arbitrary effects on the target domid.
> > 
> > With the exception of this and the privcmd layer described below,
> > DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
> > And the security aspect mentioned above won't disappear if we
> > use DMOP instead of HVMCTL. So I don't see why the hvmctl
> > series as is can't be the starting point of this, with the stability/
> > security concerns addressed subsequently, for being orthogonal.

I don't (currently) have a clear understanding of how my proposed DMOP
relates to HVMCTL.

I thought it useful to set out the DMOP proposal from first
principles, with clear motivation, discussion of not-chosen
alternatives, and of course with a clear statement of the principles
of operation and of the security design.

The security property I have quoted above is absolutely critical to
the DMOP proposal.  I'm a bit concerned by comments like the above
`with the exception of this' (which seems to refer to the security
property).

Earlier during one of the HVMCTL threads I asked

This is a slight digression, but is it intended that all of these
hvmctl's are safe to expose to a deprivileged device model process in
dom0, or to a device model stub domain ?

Jan replied:

Yes, afaict (they've been exposed the same way before).

Does that mean that functionality exposed by all the prooposed HVMCTLs
is currently available to stubdoms ?

> Yeah, to turn HVMCTL to DMOP:
> 
> 1. s/HVMCTL/DMOP/
> 2. maybe s/interface_version//

Well, that would certainly be nice.  But there are some caveats I
would like sorting out.

> >  So I don't see why the hvmctl series as is can't be the starting
> > point of this, with the stability/ security concerns addressed
> > subsequently, for being orthogonal.

Please don't misunderstand me as trying to compete with or block
your HVMCTL work.  It may well be that HVMCTL is what I want, but:

If we adopt the design principles I describe in my DMOP proposal, I
don't think the security concerns are separable.

ISTM that a patch series introducing DMOP should start with a patch
which introduces the DMOP hypercall, with no sub-operations.

Such a patch would have code content very like that in
  [PATCH 01/11] public / x86: introduce hvmctl hypercall

But, such a patch should also explain the semantics.  The Xen public
headers ought to contain explanations of the promises that the
hypervisor makes about DMOP.  Importantly:
 - the promise that a DMOP cannot harm anyone except the target domid
 - the ABI stability of the target domid field
 - what the ABI stability policy is wrt the actual DMOPs themselves

If the 01/ patch contains such promises, then logically the 02/ patch
which introduces the first DMOP is extending that promise to that
operation.  It is at that point that the security decision should be
made.

Now, there may be other ways to represent/record the security status.
But it will be necessary to either (i) avoid violating the DMOP
security promise, by making questionable calls not available via DMOP
or (ii) trying to retrofit the security promise to DMOP later.

I think (ii) is not a good approach.  It would amount to introducing a
whole new set of interfaces, and then later trying to redefine them to
have a particular security property which was not originally there.

Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 02.08.16 at 15:02,  wrote:
> On 02/08/16 12:58, Jan Beulich wrote:
> On 02.08.16 at 13:38,  wrote:
>>> On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
>>> On 01.08.16 at 13:32,  wrote:
> 4. We could invent a new hypercall `DMOP' for hypercalls which device
>models should be able to use, which always has the target domain in
>a fixed location in the arguments.  We have the dom0 privcmd driver
>know about this one hypercall number and the location of the target
>domid.
>
> Option 4 has the following advantages:
>
> * The specification of which hypercalls are authorised to qemu is
>   integrated with the specification of the hypercalls themselves:
>   There is no need to maintain a separate table which can get out of
>   step (or contain security bugs).
>
> * The changes required to the rest of the system are fairly small.
>   In particular:
>
> * We need only one small, non-varying, patch to the dom0 kernel.
>
>
> Let me flesh out option 4 in more detail:
>
>
> We define a new hypercall DMOP.
>
> Its first argument is always a target domid.  The DMOP hypercall
> number and position of the target domid in the arguments are fixed.
>
> A DMOP is defined to never put at risk the stability or security of
> the whole system, nor of the domain which calls DMOP.  However, a DMOP
> may have arbitrary effects on the target domid.

 With the exception of this and the privcmd layer described below,
 DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
 And the security aspect mentioned above won't disappear if we
 use DMOP instead of HVMCTL. So I don't see why the hvmctl
 series as is can't be the starting point of this, with the stability/
 security concerns addressed subsequently, for being orthogonal.

>>>
>>> Yeah, to turn HVMCTL to DMOP:
>>>
>>> 1. s/HVMCTL/DMOP/
>>> 2. maybe s/interface_version//
>> 
>> Andrew had brought up 2 too, but I'm really not sure that'd be a
>> good idea. I rather think we should keep it but maybe (other than
>> domctl/sysctl) recognize older versions. In any event I consider
>> having it better for an unstable interface (as Ian said, libxc is
>> supposed to provide the stable one).
> 
> A stable user space library API is no good for an in-kernel emulator,
> like that needed for Intel GVT-g -- the hypercall ABI needs to be stable.

I'm pretty certain only a (perhaps small) subset of the proposed new
operations would be needed by them, which we could then consider
marking stable.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[David Vrabel]

On 02/08/16 12:58, Jan Beulich wrote:
 On 02.08.16 at 13:38,  wrote:
>> On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
>> On 01.08.16 at 13:32,  wrote:
 4. We could invent a new hypercall `DMOP' for hypercalls which device
models should be able to use, which always has the target domain in
a fixed location in the arguments.  We have the dom0 privcmd driver
know about this one hypercall number and the location of the target
domid.

 Option 4 has the following advantages:

 * The specification of which hypercalls are authorised to qemu is
   integrated with the specification of the hypercalls themselves:
   There is no need to maintain a separate table which can get out of
   step (or contain security bugs).

 * The changes required to the rest of the system are fairly small.
   In particular:

 * We need only one small, non-varying, patch to the dom0 kernel.


 Let me flesh out option 4 in more detail:


 We define a new hypercall DMOP.

 Its first argument is always a target domid.  The DMOP hypercall
 number and position of the target domid in the arguments are fixed.

 A DMOP is defined to never put at risk the stability or security of
 the whole system, nor of the domain which calls DMOP.  However, a DMOP
 may have arbitrary effects on the target domid.
>>>
>>> With the exception of this and the privcmd layer described below,
>>> DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
>>> And the security aspect mentioned above won't disappear if we
>>> use DMOP instead of HVMCTL. So I don't see why the hvmctl
>>> series as is can't be the starting point of this, with the stability/
>>> security concerns addressed subsequently, for being orthogonal.
>>>
>>
>> Yeah, to turn HVMCTL to DMOP:
>>
>> 1. s/HVMCTL/DMOP/
>> 2. maybe s/interface_version//
> 
> Andrew had brought up 2 too, but I'm really not sure that'd be a
> good idea. I rather think we should keep it but maybe (other than
> domctl/sysctl) recognize older versions. In any event I consider
> having it better for an unstable interface (as Ian said, libxc is
> supposed to provide the stable one).

A stable user space library API is no good for an in-kernel emulator,
like that needed for Intel GVT-g -- the hypercall ABI needs to be stable.

David

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Wei Liu]

On Tue, Aug 02, 2016 at 12:42:36PM +0100, George Dunlap wrote:
> On 02/08/16 12:37, Wei Liu wrote:
> > On Mon, Aug 01, 2016 at 12:32:54PM +0100, Ian Jackson wrote:
> >> Introducing HVMCTL, Jan wrote:
> >>> A long while back separating out all control kind operations (intended
> >>> for use by only the control domain or device model) from the currect
> >>> hvmop hypercall has been discussed. This series aims at finally making
> >>> this reality (at once allowing to streamline the associated XSM checking).
> >>
> >> I think we need to introduce a new hypercall (which I will call DMOP
> >> for now) which may augment or replace some of HVMCTL.  Let me explain:
> >>
> >>
> >> We would like to be able to deprivilege qemu-in-dom0.  This is
> >> because qemu has a large attack surface and has a history of security
> >> bugs.  If we get this right we can easily reduce the impact of `guest
> >> can take over qemu' bugs to DoS; and perhaps with a bit of effort we
> >> can eliminate the DoS too.  (qemu stubdom are another way to do this
> >> but they have their own difficulties.)
> >>
> >> A part of this plan has to be a way for qemu to make hypercalls
> >> related to the guest it is servicing.  But qemu needs to be _unable_
> >> to make _other_ hypercalls.
> >>
> >> I see four possible approaches.  In IMO increasing order of
> >> desirability:
> >>
> >> 1. We could simply patch the dom0 privcmd driver to know exactly which
> >>hypercalls are permitted.  This is obviously never going to work
> >>because there would have to be a massive table in the kernel, kept
> >>in step with Xen.  We could have a kind of pattern matching engine
> >>instead, and load the tables from userspace, but that's a daft
> >>edifice to be building (even if we reuse BPF or something) and a
> >>total pain to maintain.
> >>
> >> 2. We could have some kind of privileged proxy or helper process,
> >>which makes the hypercalls on instruction from qemu.  This would be
> >>quite complicated and involve a lot of back-and-forth parameter
> >>passing.  Like option 1, this arrangement would end up embedding
> >>detailed knowledge about which hypercalls are appropriate, and have
> >>to understand all of their parameters.
> >>
> >> 3. We could have the dom0 privcmd driver wrap each of qemu's
> >>hypercalls in a special "wrap up with different XSM tag" hypercall.
> >>Then, we could specify the set of allowable hypercalls with XSM.
> >>If we want qemu deprivileged by default, this depends on turning
> >>XSM on by default.  But we want qemu depriv ASAP and there are
> >>difficulties with XSM by default.  This approach also involves
> >>writing a large and hard-to-verify hypercall permission table, in
> >>the form of an XSM policy.
> >>
> >> 4. We could invent a new hypercall `DMOP' for hypercalls which device
> >>models should be able to use, which always has the target domain in
> >>a fixed location in the arguments.  We have the dom0 privcmd driver
> >>know about this one hypercall number and the location of the target
> >>domid.
> >>
> >> Option 4 has the following advantages:
> >>
> >> * The specification of which hypercalls are authorised to qemu is
> >>   integrated with the specification of the hypercalls themselves:
> >>   There is no need to maintain a separate table which can get out of
> >>   step (or contain security bugs).
> >>
> >> * The changes required to the rest of the system are fairly small.
> >>   In particular:
> >>
> >> * We need only one small, non-varying, patch to the dom0 kernel.
> >>
> > 
> > I think your analysis makes sense.
> > 
> >>
> >> Let me flesh out option 4 in more detail:
> >>
> >>
> >> We define a new hypercall DMOP.
> >>
> >> Its first argument is always a target domid.  The DMOP hypercall
> >> number and position of the target domid in the arguments are fixed.
> >>
> >> A DMOP is defined to never put at risk the stability or security of
> >> the whole system, nor of the domain which calls DMOP.  However, a DMOP
> >> may have arbitrary effects on the target domid.
> >>
> > 
> > I would like to point out that this is non-trivial since we would need
> > to audit a lot of stuff.
> > 
> > But the requirement to audit interface is not unique to DMOP -- I expect
> > this is needed for any other approach.
> > 
> >> In the privcmd driver, we provide a new restriction ioctl, which takes
> >> a domid parameter.  After that restriction ioctl is called, the
> >> privcmd driver will permit only DMOP hypercalls, and only with the
> >> specified target domid.
> >>
> > 
> > It is phrased like that the guest kernel is supposed to enforce the
> > policy?  Would it be possible to make Xen do it? I don't think we should
> > trust DM domain kernel here.
> 
> The problem is that Xen doesn't know what process is running, and so
> can't tell whether qemuA is accessing domainA's memory, or whether qemuB
> is accessing domainA's memory.
> 
> The two options 

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 02.08.16 at 13:38,  wrote:
> On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
>> >>> On 01.08.16 at 13:32,  wrote:
>> > 4. We could invent a new hypercall `DMOP' for hypercalls which device
>> >models should be able to use, which always has the target domain in
>> >a fixed location in the arguments.  We have the dom0 privcmd driver
>> >know about this one hypercall number and the location of the target
>> >domid.
>> > 
>> > Option 4 has the following advantages:
>> > 
>> > * The specification of which hypercalls are authorised to qemu is
>> >   integrated with the specification of the hypercalls themselves:
>> >   There is no need to maintain a separate table which can get out of
>> >   step (or contain security bugs).
>> > 
>> > * The changes required to the rest of the system are fairly small.
>> >   In particular:
>> > 
>> > * We need only one small, non-varying, patch to the dom0 kernel.
>> > 
>> > 
>> > Let me flesh out option 4 in more detail:
>> > 
>> > 
>> > We define a new hypercall DMOP.
>> > 
>> > Its first argument is always a target domid.  The DMOP hypercall
>> > number and position of the target domid in the arguments are fixed.
>> > 
>> > A DMOP is defined to never put at risk the stability or security of
>> > the whole system, nor of the domain which calls DMOP.  However, a DMOP
>> > may have arbitrary effects on the target domid.
>> 
>> With the exception of this and the privcmd layer described below,
>> DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
>> And the security aspect mentioned above won't disappear if we
>> use DMOP instead of HVMCTL. So I don't see why the hvmctl
>> series as is can't be the starting point of this, with the stability/
>> security concerns addressed subsequently, for being orthogonal.
>> 
> 
> Yeah, to turn HVMCTL to DMOP:
> 
> 1. s/HVMCTL/DMOP/
> 2. maybe s/interface_version//

Andrew had brought up 2 too, but I'm really not sure that'd be a
good idea. I rather think we should keep it but maybe (other than
domctl/sysctl) recognize older versions. In any event I consider
having it better for an unstable interface (as Ian said, libxc is
supposed to provide the stable one).

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[George Dunlap]

On 02/08/16 12:37, Wei Liu wrote:
> On Mon, Aug 01, 2016 at 12:32:54PM +0100, Ian Jackson wrote:
>> Introducing HVMCTL, Jan wrote:
>>> A long while back separating out all control kind operations (intended
>>> for use by only the control domain or device model) from the currect
>>> hvmop hypercall has been discussed. This series aims at finally making
>>> this reality (at once allowing to streamline the associated XSM checking).
>>
>> I think we need to introduce a new hypercall (which I will call DMOP
>> for now) which may augment or replace some of HVMCTL.  Let me explain:
>>
>>
>> We would like to be able to deprivilege qemu-in-dom0.  This is
>> because qemu has a large attack surface and has a history of security
>> bugs.  If we get this right we can easily reduce the impact of `guest
>> can take over qemu' bugs to DoS; and perhaps with a bit of effort we
>> can eliminate the DoS too.  (qemu stubdom are another way to do this
>> but they have their own difficulties.)
>>
>> A part of this plan has to be a way for qemu to make hypercalls
>> related to the guest it is servicing.  But qemu needs to be _unable_
>> to make _other_ hypercalls.
>>
>> I see four possible approaches.  In IMO increasing order of
>> desirability:
>>
>> 1. We could simply patch the dom0 privcmd driver to know exactly which
>>hypercalls are permitted.  This is obviously never going to work
>>because there would have to be a massive table in the kernel, kept
>>in step with Xen.  We could have a kind of pattern matching engine
>>instead, and load the tables from userspace, but that's a daft
>>edifice to be building (even if we reuse BPF or something) and a
>>total pain to maintain.
>>
>> 2. We could have some kind of privileged proxy or helper process,
>>which makes the hypercalls on instruction from qemu.  This would be
>>quite complicated and involve a lot of back-and-forth parameter
>>passing.  Like option 1, this arrangement would end up embedding
>>detailed knowledge about which hypercalls are appropriate, and have
>>to understand all of their parameters.
>>
>> 3. We could have the dom0 privcmd driver wrap each of qemu's
>>hypercalls in a special "wrap up with different XSM tag" hypercall.
>>Then, we could specify the set of allowable hypercalls with XSM.
>>If we want qemu deprivileged by default, this depends on turning
>>XSM on by default.  But we want qemu depriv ASAP and there are
>>difficulties with XSM by default.  This approach also involves
>>writing a large and hard-to-verify hypercall permission table, in
>>the form of an XSM policy.
>>
>> 4. We could invent a new hypercall `DMOP' for hypercalls which device
>>models should be able to use, which always has the target domain in
>>a fixed location in the arguments.  We have the dom0 privcmd driver
>>know about this one hypercall number and the location of the target
>>domid.
>>
>> Option 4 has the following advantages:
>>
>> * The specification of which hypercalls are authorised to qemu is
>>   integrated with the specification of the hypercalls themselves:
>>   There is no need to maintain a separate table which can get out of
>>   step (or contain security bugs).
>>
>> * The changes required to the rest of the system are fairly small.
>>   In particular:
>>
>> * We need only one small, non-varying, patch to the dom0 kernel.
>>
> 
> I think your analysis makes sense.
> 
>>
>> Let me flesh out option 4 in more detail:
>>
>>
>> We define a new hypercall DMOP.
>>
>> Its first argument is always a target domid.  The DMOP hypercall
>> number and position of the target domid in the arguments are fixed.
>>
>> A DMOP is defined to never put at risk the stability or security of
>> the whole system, nor of the domain which calls DMOP.  However, a DMOP
>> may have arbitrary effects on the target domid.
>>
> 
> I would like to point out that this is non-trivial since we would need
> to audit a lot of stuff.
> 
> But the requirement to audit interface is not unique to DMOP -- I expect
> this is needed for any other approach.
> 
>> In the privcmd driver, we provide a new restriction ioctl, which takes
>> a domid parameter.  After that restriction ioctl is called, the
>> privcmd driver will permit only DMOP hypercalls, and only with the
>> specified target domid.
>>
> 
> It is phrased like that the guest kernel is supposed to enforce the
> policy?  Would it be possible to make Xen do it? I don't think we should
> trust DM domain kernel here.

The problem is that Xen doesn't know what process is running, and so
can't tell whether qemuA is accessing domainA's memory, or whether qemuB
is accessing domainA's memory.

The two options that have been proposed are:

1. Have a way for dom0 to give Xen an XSM tag for the current process
(so Xen can do the enforcing)

2. Have dom0 filter out the calls based on the fact that all the
hypercalls have the same template (i.e., domid in the same position).

Either 

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Wei Liu]

On Mon, Aug 01, 2016 at 12:32:54PM +0100, Ian Jackson wrote:
> Introducing HVMCTL, Jan wrote:
> > A long while back separating out all control kind operations (intended
> > for use by only the control domain or device model) from the currect
> > hvmop hypercall has been discussed. This series aims at finally making
> > this reality (at once allowing to streamline the associated XSM checking).
> 
> I think we need to introduce a new hypercall (which I will call DMOP
> for now) which may augment or replace some of HVMCTL.  Let me explain:
> 
> 
> We would like to be able to deprivilege qemu-in-dom0.  This is
> because qemu has a large attack surface and has a history of security
> bugs.  If we get this right we can easily reduce the impact of `guest
> can take over qemu' bugs to DoS; and perhaps with a bit of effort we
> can eliminate the DoS too.  (qemu stubdom are another way to do this
> but they have their own difficulties.)
> 
> A part of this plan has to be a way for qemu to make hypercalls
> related to the guest it is servicing.  But qemu needs to be _unable_
> to make _other_ hypercalls.
> 
> I see four possible approaches.  In IMO increasing order of
> desirability:
> 
> 1. We could simply patch the dom0 privcmd driver to know exactly which
>hypercalls are permitted.  This is obviously never going to work
>because there would have to be a massive table in the kernel, kept
>in step with Xen.  We could have a kind of pattern matching engine
>instead, and load the tables from userspace, but that's a daft
>edifice to be building (even if we reuse BPF or something) and a
>total pain to maintain.
> 
> 2. We could have some kind of privileged proxy or helper process,
>which makes the hypercalls on instruction from qemu.  This would be
>quite complicated and involve a lot of back-and-forth parameter
>passing.  Like option 1, this arrangement would end up embedding
>detailed knowledge about which hypercalls are appropriate, and have
>to understand all of their parameters.
> 
> 3. We could have the dom0 privcmd driver wrap each of qemu's
>hypercalls in a special "wrap up with different XSM tag" hypercall.
>Then, we could specify the set of allowable hypercalls with XSM.
>If we want qemu deprivileged by default, this depends on turning
>XSM on by default.  But we want qemu depriv ASAP and there are
>difficulties with XSM by default.  This approach also involves
>writing a large and hard-to-verify hypercall permission table, in
>the form of an XSM policy.
> 
> 4. We could invent a new hypercall `DMOP' for hypercalls which device
>models should be able to use, which always has the target domain in
>a fixed location in the arguments.  We have the dom0 privcmd driver
>know about this one hypercall number and the location of the target
>domid.
> 
> Option 4 has the following advantages:
> 
> * The specification of which hypercalls are authorised to qemu is
>   integrated with the specification of the hypercalls themselves:
>   There is no need to maintain a separate table which can get out of
>   step (or contain security bugs).
> 
> * The changes required to the rest of the system are fairly small.
>   In particular:
> 
> * We need only one small, non-varying, patch to the dom0 kernel.
> 

I think your analysis makes sense.

> 
> Let me flesh out option 4 in more detail:
> 
> 
> We define a new hypercall DMOP.
> 
> Its first argument is always a target domid.  The DMOP hypercall
> number and position of the target domid in the arguments are fixed.
> 
> A DMOP is defined to never put at risk the stability or security of
> the whole system, nor of the domain which calls DMOP.  However, a DMOP
> may have arbitrary effects on the target domid.
> 

I would like to point out that this is non-trivial since we would need
to audit a lot of stuff.

But the requirement to audit interface is not unique to DMOP -- I expect
this is needed for any other approach.

> In the privcmd driver, we provide a new restriction ioctl, which takes
> a domid parameter.  After that restriction ioctl is called, the
> privcmd driver will permit only DMOP hypercalls, and only with the
> specified target domid.
> 

It is phrased like that the guest kernel is supposed to enforce the
policy?  Would it be possible to make Xen do it? I don't think we should
trust DM domain kernel here.

Wei.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Wei Liu]

On Mon, Aug 01, 2016 at 06:41:20AM -0600, Jan Beulich wrote:
> >>> On 01.08.16 at 13:32,  wrote:
> > 4. We could invent a new hypercall `DMOP' for hypercalls which device
> >models should be able to use, which always has the target domain in
> >a fixed location in the arguments.  We have the dom0 privcmd driver
> >know about this one hypercall number and the location of the target
> >domid.
> > 
> > Option 4 has the following advantages:
> > 
> > * The specification of which hypercalls are authorised to qemu is
> >   integrated with the specification of the hypercalls themselves:
> >   There is no need to maintain a separate table which can get out of
> >   step (or contain security bugs).
> > 
> > * The changes required to the rest of the system are fairly small.
> >   In particular:
> > 
> > * We need only one small, non-varying, patch to the dom0 kernel.
> > 
> > 
> > Let me flesh out option 4 in more detail:
> > 
> > 
> > We define a new hypercall DMOP.
> > 
> > Its first argument is always a target domid.  The DMOP hypercall
> > number and position of the target domid in the arguments are fixed.
> > 
> > A DMOP is defined to never put at risk the stability or security of
> > the whole system, nor of the domain which calls DMOP.  However, a DMOP
> > may have arbitrary effects on the target domid.
> 
> With the exception of this and the privcmd layer described below,
> DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
> And the security aspect mentioned above won't disappear if we
> use DMOP instead of HVMCTL. So I don't see why the hvmctl
> series as is can't be the starting point of this, with the stability/
> security concerns addressed subsequently, for being orthogonal.
> 

Yeah, to turn HVMCTL to DMOP:

1. s/HVMCTL/DMOP/
2. maybe s/interface_version//

I think we could at least do #1 and merge the series.

Wei.

> Jan
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] Device model operation hypercall (DMOP, re qemu depriv)

[Jan Beulich]

>>> On 01.08.16 at 13:32,  wrote:
> 4. We could invent a new hypercall `DMOP' for hypercalls which device
>models should be able to use, which always has the target domain in
>a fixed location in the arguments.  We have the dom0 privcmd driver
>know about this one hypercall number and the location of the target
>domid.
> 
> Option 4 has the following advantages:
> 
> * The specification of which hypercalls are authorised to qemu is
>   integrated with the specification of the hypercalls themselves:
>   There is no need to maintain a separate table which can get out of
>   step (or contain security bugs).
> 
> * The changes required to the rest of the system are fairly small.
>   In particular:
> 
> * We need only one small, non-varying, patch to the dom0 kernel.
> 
> 
> Let me flesh out option 4 in more detail:
> 
> 
> We define a new hypercall DMOP.
> 
> Its first argument is always a target domid.  The DMOP hypercall
> number and position of the target domid in the arguments are fixed.
> 
> A DMOP is defined to never put at risk the stability or security of
> the whole system, nor of the domain which calls DMOP.  However, a DMOP
> may have arbitrary effects on the target domid.

With the exception of this and the privcmd layer described below,
DMOP == HVMCTL afaics. The privcmd layer is independent anyway.
And the security aspect mentioned above won't disappear if we
use DMOP instead of HVMCTL. So I don't see why the hvmctl
series as is can't be the starting point of this, with the stability/
security concerns addressed subsequently, for being orthogonal.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel