>>> On 09.12.16 at 16:44, <ian.jack...@eu.citrix.com> wrote:
> This will allow us to keep track of the total amount of work we are
> doing. When it becomes excessive, we mark the ELF broken, and stop
> processing.
>
> This is a more robust way of preventing DoS problems by bad images
> than attempting to prove, for each of the (sometimes rather deeply
> nested) loops, that the total work is "reasonable". We bound the
> notional work by 4x the image size (plus 1M).
The 4x has been selected arbitrarily, I suppose?
> --- a/xen/common/libelf/libelf-loader.c
> +++ b/xen/common/libelf/libelf-loader.c
> @@ -38,6 +38,7 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char
> *image_input, size_t
> ELF_HANDLE_DECL(elf_shdr) shdr;
> unsigned i, count, section, link;
> uint64_t offset;
> + const uint64_t max_size_for_deacc = (1UL << 63)/ELF_MAX_ITERATION_FACTOR;
This will need adjustment for 32-bit tool stack builds - did you
perhaps mean UINT64_C(1), considering the type of the variable?
Also please add blanks around the / .
> @@ -546,6 +551,15 @@ uint64_t elf_lookup_addr(struct elf_binary * elf, const
> char *symbol)
> return value;
> }
>
> +bool elf_iter_ok_counted(struct elf_binary *elf, uint64_t maxcopysz) {
> + if (maxcopysz > elf->iteration_deaccumulator)
> + elf_mark_broken(elf, "excessive iteration - too much work to parse");
> + if (elf->broken)
> + return false;
> + elf->iteration_deaccumulator -= maxcopysz;
> + return true;
> +}
Hypervisor coding style here please (missing lots of blanks, and the
opening brace needs to go on its own line). I think there are more
such style problems further down.
And then - would it perhaps make sense to have most of this
function's body in #ifndef __XEN__, as there's nothing to guard
against when loading Dom0?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
