Re: [Xen-devel] [PATCH v6 08/24] xsplice: Add helper elf routines

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/04/16 17:19, Ian Jackson wrote:
> Konrad Rzeszutek Wilk writes ("[PATCH v6 08/24] xsplice: Add helper elf 
> routines"):
>> From: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>
>> Add Elf routines and data structures in preparation for loading an
>> xSplice payload.
>>
>> We make an assumption that the max number of sections an ELF payload
>> can have is 64. We can in future make this be dependent on the
>> names of the sections and verifying against a list, but for right now
>> this suffices.
>>
>> Also we a whole lot of checks to make sure that the ELF payload
>> file is not corrupted nor that the offsets point past the file.
> This is good, but: ideally I would like to avoid conducting a detailed
> security review of this code.
>
> My understanding of this is that the purpose of this machinery is to
> supply binary runtime patches to the hypervisor.  So I think someone
> who can inject malicious xsplice payloads can already control the
> host.  Is that right ?

Correct.

>
> If so then bugs in this loader cannot be any security impact.

I agree.

The reason for the checks is so Xen doesn't accidentally fall over a
malformed ELF.  Earlier versions of this patch were a bit too lax in
trusting the integrity of the ELF image for my liking, which is why I
specifically asked for better verification.

> It might be worth mentioning somewhere that this loader must not be
> used for xsplice payloads for guest kernels.

I don't see how this is related.  If the host admin wanted to patch
guest kernels without using the kernels internal self-patching
mechanism, it would be infinitely easier to do the patching from dom0,
using toolstack mapping powers.

~Andrew

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