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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monné writes ("Re: [PATCH v4 1/6] libelf: rewrite symtab/strtab 
loading for Dom0"):
> 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"):
> >>  #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.

Aha.  Hmm.

The problem is that someone might use this macro to access a structure
that was _not_ stored on the stack.

...

Oh, I see, you have changed the type of _hdr from an ELF_MAKE_HANDLE
to an actual literal structure.  So actually this change is correct.
I think this ought to be (have been) mentioned in the commit message.

> 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.

elf_store_val calls elf_access_ok before calling elf_memcpy_unchecked.

It wouldn't need `handlers', it would need the struct being registered
as the xdest.  But there is no need if it never needs to be accessed
through any kind of guest-influenceable pointers.

Ian.

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