Re: [Xen-devel] [PATCH] xen: arm: fully implement multicall interface.

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2014-04-01 at 10:05 +0100, Julien Grall wrote:
> 
> On 31/03/14 17:38, George Dunlap wrote:
> > On 03/28/2014 03:07 PM, Ian Campbell wrote:
> >> On Fri, 2014-03-28 at 15:01 +0000, Julien Grall wrote:
> >>> On 03/28/2014 02:45 PM, Ian Campbell wrote:
> >>>
> >>>> My feeling is that any (exploitable or otherwise) issue due to this
> >>>> would be due to lack of proper error checking in the hypercall, and
> >>>> would be equally accessible by a 64-bit guest.
> >>> I don't think it's exploitable. IHMO, the main point is to give a useful
> >>> debug to the user rather than an obscure error message because the given
> >>> pointer is invalid (perhaps by mistake).
> >> Right.
> >>
> >> We also want to avoid the case where it just happens to work in one
> >> configuration (i.e. 32-on-32) but fails in another (i.e. 32-on-64).
> >>
> >>>> I'm considering whether to add an #ifndef NDEBUG check here which will
> >>>> reject a multicall from a 32-bit guest where any of the arguments
> >>>> (arm_hypercall_table[nr].nr_args) are non-zero in their top 32-bit. I
> >>>> can't decide whether -EINVAL or domain_kill() would be more
> >>>> appropriate.
> >>>> I'm actually leaning towards the latter.
> >>>>
> >>>> Thoughts?
> >>> Killing the domain is a bit tough.
> >> Sure, but the point is to flush out 32-bit guests which make invalid
> >> assumptions which will be broken when they run on 64-bit vs. 32-bit Xen.
> >
> > Perhaps return -EINVAL for NDEBUG, and kill if !NDEBUG?
> >
> > We generally don't want to be killing production VMs unless absolutely
> > necessary.  One would of course hope that we had caught any bugs during
> > development, but things don't always work the way you'd think...

More of often than not I'd expect that the guest would shoot itself if
one of these hypercalls failed.

But the approach you suggest is probably the best compromise.

> Out-of-context, I've noticed that most of trap failure will kill the 
> domain. From the ARM ARM , if a coprocessor instruction is failing, we 
> should        generate an Undefined Instruction exception (see P.7.5).

You mean HSR_EC_CP15_32, HSR_EC_CP15_64 and HSR_EC_SYSREG?

I've not checked but I think we only ask for traps for things which we
are supposed to be able to handle, so anything which is trapped which we
can't handle is a bug and would indicate the guest doing something
funky.

Now maybe that is wrong and we are asking for traps which we don't
handle, in which case we should either handle them or stop asking for
them.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel