Re: [Xen-devel] [PATCH v4 1/6] libelf: rewrite symtab/strtab loading for Dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.01.16 at 18:55, <roger.pau@xxxxxxxxxx> wrote:
> El 21/01/16 a les 18.29, Ian Jackson ha escrit:
>> Roger Pau Monne writes ("[PATCH v4 1/6] libelf: rewrite symtab/strtab 
> loading for Dom0"):
>>> Current implementation of elf_load_bsdsyms is broken when loading inside of
>>> a HVM guest, because it assumes elf_memcpy_safe is able to write into guest
>>> memory space, which it is not.
>>>
>>> Take the oportunity to do some cleanup and properly document how
>>> elf_{parse/load}_bsdsyms works. The new implementation uses elf_load_image
>>> when dealing with data that needs to be copied to the guest memory space.
>>> Also reduce the number of section headers copied to the minimum necessary.
>> ...
>>>  #define elf_hdr_elm(_elf, _hdr, _elm, _val)     \
>>>  do {                                            \
>>>      if ( elf_64bit(_elf) )                      \
>>> -        elf_store_field(_elf, _hdr, e64._elm, _val);  \
>>> +        (_hdr).e64._elm = _val;                 \
>> 
>> This seems to bypass the safe store mechanism which was introduced to
>> fix XSA-55.
> 
> This macro is only used to store fields inside of a structure that's
> allocated on the stack, and it doesn't involve any kind of pointer
> magic/arithmetic. The way it was used previously in this function indeed
> required the use of the _safe mechanism in order to prevent writing into
> arbitrary memory places, because we were actually modifying guest memory
> IIRC.
> 
> I could restore the previous behaviour, but that would mean adding some
> handlers in order to access the structure. Since this is only used for
> Dom0, which already makes use of the elf_memcpy_unchecked function as
> called by elf_store_val which is in turn called from elf_store_field, so
> it's not like we were protected previously anyway.

If this is indeed Dom0-only code, could (and perhaps should) it be
guarded suitably by an #ifdef to make obvious it's not used for
DomU loading, and hence not security sensitive? From looking at
the call sites of elf_{parse,load}_bsdsyms() I can't, btw., tell that
this is Dom0-only ...

Jan


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