Messages in this thread | | | Date | Tue, 8 Mar 2016 13:30:05 +0800 | From | Baoquan He <> | Subject | Re: [PATCH v3 11/19] x86, boot: Add checking for memcpy |
| |
On 03/07/16 at 03:36pm, Kees Cook wrote: > On Fri, Mar 4, 2016 at 8:25 AM, Baoquan He <bhe@redhat.com> wrote: > > From: Yinghai Lu <yinghai@kernel.org> > > > > parse_elf is using local memcpy to move section to running position. > > That memcpy actually only support no overlapping case or when dest < src. > > > > Add checking in memcpy to find out the wrong case for future use, at > > that time we will need to have backward memcpy for it. > > > > Also add comments in parse_elf about the fact. > > Seems like this would be better to just fix the memcpy to handle the overlap?
Yeah, agree. I will remove the code comment.
> > -Kees > > > > > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > > --- > > v2->v3: > > Add a declaration for error() since its declaration is in misc.h. > > But it's not included in compressed/string.c. > > > > arch/x86/boot/compressed/misc.c | 14 +++++++------- > > arch/x86/boot/compressed/misc.h | 2 ++ > > arch/x86/boot/compressed/string.c | 29 +++++++++++++++++++++++++++-- > > 3 files changed, 36 insertions(+), 9 deletions(-) > > > > diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c > > index dd7ed8a..4b2cd0c 100644 > > --- a/arch/x86/boot/compressed/misc.c > > +++ b/arch/x86/boot/compressed/misc.c > > @@ -114,9 +114,6 @@ > > #undef memset > > #define memzero(s, n) memset((s), 0, (n)) > > > > - > > -static void error(char *m); > > - > > /* > > * This is set up by the setup-routine at boot-time > > */ > > @@ -243,7 +240,7 @@ void __puthex(unsigned long value) > > } > > } > > > > -static void error(char *x) > > +void error(char *x) > > { > > error_putstr("\n\n"); > > error_putstr(x); > > @@ -378,9 +375,12 @@ static void parse_elf(void *output) > > #else > > dest = (void *)(phdr->p_paddr); > > #endif > > - memcpy(dest, > > - output + phdr->p_offset, > > - phdr->p_filesz); > > + /* > > + * simple version memcpy only can work when dest is > > + * smaller than src or no overlapping. > > + * Here dest is smaller than src always. > > + */ > > + memcpy(dest, output + phdr->p_offset, phdr->p_filesz); > > break; > > default: /* Ignore other PT_* */ break; > > } > > diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h > > index 11736a6..39d0e9a 100644 > > --- a/arch/x86/boot/compressed/misc.h > > +++ b/arch/x86/boot/compressed/misc.h > > @@ -38,6 +38,8 @@ void __puthex(unsigned long value); > > #define error_putstr(__x) __putstr(__x) > > #define error_puthex(__x) __puthex(__x) > > > > +void error(char *x); > > + > > #ifdef CONFIG_X86_VERBOSE_BOOTUP > > > > #define debug_putstr(__x) __putstr(__x) > > diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c > > index 00e788b..3a935d0 100644 > > --- a/arch/x86/boot/compressed/string.c > > +++ b/arch/x86/boot/compressed/string.c > > @@ -1,7 +1,7 @@ > > #include "../string.c" > > > > #ifdef CONFIG_X86_32 > > -void *memcpy(void *dest, const void *src, size_t n) > > +void *__memcpy(void *dest, const void *src, size_t n) > > { > > int d0, d1, d2; > > asm volatile( > > @@ -15,7 +15,7 @@ void *memcpy(void *dest, const void *src, size_t n) > > return dest; > > } > > #else > > -void *memcpy(void *dest, const void *src, size_t n) > > +void *__memcpy(void *dest, const void *src, size_t n) > > { > > long d0, d1, d2; > > asm volatile( > > @@ -30,6 +30,31 @@ void *memcpy(void *dest, const void *src, size_t n) > > } > > #endif > > > > +extern void error(char *x); > > +void *memcpy(void *dest, const void *src, size_t n) > > +{ > > + unsigned long start_dest, end_dest; > > + unsigned long start_src, end_src; > > + unsigned long max_start, min_end; > > + > > + if (dest < src) > > + return __memcpy(dest, src, n); > > + > > + start_dest = (unsigned long)dest; > > + end_dest = (unsigned long)dest + n; > > + start_src = (unsigned long)src; > > + end_src = (unsigned long)src + n; > > + max_start = (start_dest > start_src) ? start_dest : start_src; > > + min_end = (end_dest < end_src) ? end_dest : end_src; > > + > > + if (max_start >= min_end) > > + return __memcpy(dest, src, n); > > + > > + error("memcpy does not support overlapping with dest > src!\n"); > > + > > + return dest; > > +} > > + > > void *memset(void *s, int c, size_t n) > > { > > int i; > > -- > > 2.5.0 > > > > > > -- > Kees Cook > Chrome OS & Brillo Security
| |