Re: [Xen-devel] [PATCH 1/8] libelf: loop safety: Introduce elf_iter_ok and elf_strcmp_safe

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 09.12.16 at 16:44, <ian.jackson@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel