| Message ID | 1481298289-13546-5-git-send-email-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 09.12.16 at 16:44, <ian.jackson@eu.citrix.com> wrote: > @@ -80,7 +81,8 @@ void elf_memcpy_safe(struct elf_binary *elf, elf_ptrval dst, > > void elf_memset_safe(struct elf_binary *elf, elf_ptrval dst, int c, size_t size) > { > - if ( elf_access_ok(elf, dst, size) ) > + if ( elf_access_ok(elf, dst, size) && > + elf_iter_ok_counted(elf, size)) With the style issue here fixed, Acked-by: Jan Beulich <jbeulich@suse.com> despite not being really happy about the added clutter. Jan
Jan Beulich writes ("Re: [PATCH 4/8] libelf: loop safety: Call elf_iter_ok_counted at every *mem*_unsafe"):
> On 09.12.16 at 16:44, <ian.jackson@eu.citrix.com> wrote:
> > - if ( elf_access_ok(elf, dst, size) )
> > + if ( elf_access_ok(elf, dst, size) &&
> > + elf_iter_ok_counted(elf, size))
>
> With the style issue here fixed,
> Acked-by: Jan Beulich <jbeulich@suse.com>
> despite not being really happy about the added clutter.
I have added the missing space, and also moved the && to the next
line (as seems to be done in much of the hypervisor).
I hope that meets with your approval.
Ian.
>>> On 12.12.16 at 16:54, <ian.jackson@eu.citrix.com> wrote: > Jan Beulich writes ("Re: [PATCH 4/8] libelf: loop safety: Call > elf_iter_ok_counted at every *mem*_unsafe"): >> On 09.12.16 at 16:44, <ian.jackson@eu.citrix.com> wrote: >> > - if ( elf_access_ok(elf, dst, size) ) >> > + if ( elf_access_ok(elf, dst, size) && >> > + elf_iter_ok_counted(elf, size)) >> >> With the style issue here fixed, >> Acked-by: Jan Beulich <jbeulich@suse.com> >> despite not being really happy about the added clutter. > > I have added the missing space, and also moved the && to the next > line (as seems to be done in much of the hypervisor). > > I hope that meets with your approval. Well, I don't mind either placement of the && (personally I like it better at the front of a line, but the common model in fact is to have it at the end). Jan
Jan Beulich writes ("Re: [PATCH 4/8] libelf: loop safety: Call elf_iter_ok_counted at every *mem*_unsafe"):
> On 12.12.16 at 16:54, <ian.jackson@eu.citrix.com> wrote:
> > I have added the missing space, and also moved the && to the next
> > line (as seems to be done in much of the hypervisor).
> >
> > I hope that meets with your approval.
>
> Well, I don't mind either placement of the && (personally I like it
> better at the front of a line, but the common model in fact is to
> have it at the end).
Heh. At least we agree on something here then ...
Ian.
diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c index b139e32..87a47d9 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -498,6 +498,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, unsigned total_note_count = 0; elf_memset_unchecked(parms, 0, sizeof(*parms)); + elf_iter_ok_counted(elf, sizeof(*parms)); parms->virt_base = UNSET_ADDR; parms->virt_entry = UNSET_ADDR; parms->virt_hypercall = UNSET_ADDR; diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c index 68c9021..d5e51d3 100644 --- a/xen/common/libelf/libelf-loader.c +++ b/xen/common/libelf/libelf-loader.c @@ -46,7 +46,7 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t return -1; } - elf_memset_unchecked(elf, 0, sizeof(*elf)); + elf_memset_unchecked(elf, 0, sizeof(*elf)); /* loop safety: singleton */ elf->image_base = image_input; elf->size = size; elf->ehdr = ELF_MAKE_HANDLE(elf_ehdr, (elf_ptrval)image_input); diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c index 56dab63..ab83150 100644 --- a/xen/common/libelf/libelf-tools.c +++ b/xen/common/libelf/libelf-tools.c @@ -69,7 +69,8 @@ void elf_memcpy_safe(struct elf_binary *elf, elf_ptrval dst, elf_ptrval src, size_t size) { if ( elf_access_ok(elf, dst, size) && - elf_access_ok(elf, src, size) ) + elf_access_ok(elf, src, size) && + elf_iter_ok_counted(elf, size) ) { /* use memmove because these checks do not prove that the * regions don't overlap and overlapping regions grant @@ -80,7 +81,8 @@ void elf_memcpy_safe(struct elf_binary *elf, elf_ptrval dst, void elf_memset_safe(struct elf_binary *elf, elf_ptrval dst, int c, size_t size) { - if ( elf_access_ok(elf, dst, size) ) + if ( elf_access_ok(elf, dst, size) && + elf_iter_ok_counted(elf, size)) { elf_memset_unchecked(ELF_UNSAFE_PTR(dst), c, size); }
When we use elf_mem*_unsafe, we need to check that we are not doing too much work. Ensure that a call to elf_iter_ok_counted is near every call to elf_mem*_unsafe. (At one call site, just have a comment instead.) Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> --- xen/common/libelf/libelf-dominfo.c | 1 + xen/common/libelf/libelf-loader.c | 2 +- xen/common/libelf/libelf-tools.c | 6 ++++-- 3 files changed, 6 insertions(+), 3 deletions(-)