Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1390647 > unrolled thread

[PATCH v3] x86/boot: Warn on future overlapping memcpy() use

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] x86/boot: Warn on future overlapping memcpy() use Kees Cook <keescook@chromium.org> - 2016-04-29 01:50 +0200
    Re: [PATCH v3] x86/boot: Warn on future overlapping memcpy() use Kees Cook <keescook@chromium.org> - 2016-04-29 01:50 +0200
    Re: [PATCH v3] x86/boot: Warn on future overlapping memcpy() use kbuild test robot <lkp@intel.com> - 2016-04-29 02:00 +0200
      Re: [PATCH v3] x86/boot: Warn on future overlapping memcpy() use Kees Cook <keescook@chromium.org> - 2016-04-29 02:20 +0200

#1390647 — [PATCH v3] x86/boot: Warn on future overlapping memcpy() use

If an overlapping memcpy() is ever attempted, we should report it and
gracefully call memmove(). These cases can be found and fixed to use
memmove() correctly, but in the meantime, we will not break booting.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/string.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c
index 2befeca1aada..7402227fdfdb 100644
--- a/arch/x86/boot/compressed/string.c
+++ b/arch/x86/boot/compressed/string.c
@@ -8,7 +8,7 @@
 #include "../string.c"
 
 #ifdef CONFIG_X86_32
-void *memcpy(void *dest, const void *src, size_t n)
+static void *__memcpy(void *dest, const void *src, size_t n)
 {
 	int d0, d1, d2;
 	asm volatile(
@@ -22,7 +22,7 @@ void *memcpy(void *dest, const void *src, size_t n)
 	return dest;
 }
 #else
-void *memcpy(void *dest, const void *src, size_t n)
+static void *__memcpy(void *dest, const void *src, size_t n)
 {
 	long d0, d1, d2;
 	asm volatile(
@@ -60,3 +60,14 @@ void *memmove(void *dest, const void *src, size_t n)
 
 	return dest;
 }
+
+/* Detect and warn about potential overlaps, but handle them with memmove. */
+void *memcpy(void *dest, const void *src, size_t n)
+{
+	if (dest > src && dest - src < n) {
+		warn("Avoiding potentially unsafe overlapping memcpy()!");
+		return memmove(dest, src, n);
+	}
+	return __memcpy(dest, src, n);
+}
+
-- 
2.6.3


-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [next] | [standalone]

#1390650

(Sorry, this v3 got sent to an incomplete CC list...)


On Thu, Apr 28, 2016 at 4:46 PM, Kees Cook <keescook@chromium.org> wrote:
> If an overlapping memcpy() is ever attempted, we should report it and
> gracefully call memmove(). These cases can be found and fixed to use
> memmove() correctly, but in the meantime, we will not break booting.
>
> Suggested-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  arch/x86/boot/compressed/string.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/string.c b/arch/x86/boot/compressed/string.c
> index 2befeca1aada..7402227fdfdb 100644
> --- a/arch/x86/boot/compressed/string.c
> +++ b/arch/x86/boot/compressed/string.c
> @@ -8,7 +8,7 @@
>  #include "../string.c"
>
>  #ifdef CONFIG_X86_32
> -void *memcpy(void *dest, const void *src, size_t n)
> +static void *__memcpy(void *dest, const void *src, size_t n)
>  {
>         int d0, d1, d2;
>         asm volatile(
> @@ -22,7 +22,7 @@ void *memcpy(void *dest, const void *src, size_t n)
>         return dest;
>  }
>  #else
> -void *memcpy(void *dest, const void *src, size_t n)
> +static void *__memcpy(void *dest, const void *src, size_t n)
>  {
>         long d0, d1, d2;
>         asm volatile(
> @@ -60,3 +60,14 @@ void *memmove(void *dest, const void *src, size_t n)
>
>         return dest;
>  }
> +
> +/* Detect and warn about potential overlaps, but handle them with memmove. */
> +void *memcpy(void *dest, const void *src, size_t n)
> +{
> +       if (dest > src && dest - src < n) {
> +               warn("Avoiding potentially unsafe overlapping memcpy()!");
> +               return memmove(dest, src, n);
> +       }
> +       return __memcpy(dest, src, n);
> +}
> +
> --
> 2.6.3
>
>
> --
> Kees Cook
> Chrome OS & Brillo Security



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1390652

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test ERROR on tip/auto-latest]
[cannot apply to tip/x86/core v4.6-rc5 next-20160428]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Kees-Cook/x86-boot-Warn-on-future-overlapping-memcpy-use/20160429-075026
config: i386-tinyconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   arch/x86/boot/compressed/string.c: In function 'memmove':
>> arch/x86/boot/compressed/string.c:56:10: warning: implicit declaration of function 'memcpy' [-Wimplicit-function-declaration]
      return memcpy(dest, src, n);
             ^
>> arch/x86/boot/compressed/string.c:56:10: warning: return makes pointer from integer without a cast [-Wint-conversion]
   arch/x86/boot/compressed/string.c: At top level:
>> arch/x86/boot/compressed/string.c:65:7: error: conflicting types for 'memcpy'
    void *memcpy(void *dest, const void *src, size_t n)
          ^
   arch/x86/boot/compressed/string.c:56:10: note: previous implicit declaration of 'memcpy' was here
      return memcpy(dest, src, n);
             ^
   arch/x86/boot/compressed/string.c: In function 'memcpy':
>> arch/x86/boot/compressed/string.c:68:3: warning: implicit declaration of function 'warn' [-Wimplicit-function-declaration]
      warn("Avoiding potentially unsafe overlapping memcpy()!");
      ^

vim +/memcpy +65 arch/x86/boot/compressed/string.c

    50	void *memmove(void *dest, const void *src, size_t n)
    51	{
    52		unsigned char *d = dest;
    53		const unsigned char *s = src;
    54	
    55		if (d <= s || d - s >= n)
  > 56			return memcpy(dest, src, n);
    57	
    58		while (n-- > 0)
    59			d[n] = s[n];
    60	
    61		return dest;
    62	}
    63	
    64	/* Detect and warn about potential overlaps, but handle them with memmove. */
  > 65	void *memcpy(void *dest, const void *src, size_t n)
    66	{
    67		if (dest > src && dest - src < n) {
  > 68			warn("Avoiding potentially unsafe overlapping memcpy()!");
    69			return memmove(dest, src, n);
    70		}
    71		return __memcpy(dest, src, n);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1390666

Argh. I'm hating this patch. :)

Will fix with v4.

On Thu, Apr 28, 2016 at 4:56 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi,
>
> [auto build test ERROR on tip/auto-latest]
> [cannot apply to tip/x86/core v4.6-rc5 next-20160428]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Kees-Cook/x86-boot-Warn-on-future-overlapping-memcpy-use/20160429-075026
> config: i386-tinyconfig (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
>    arch/x86/boot/compressed/string.c: In function 'memmove':
>>> arch/x86/boot/compressed/string.c:56:10: warning: implicit declaration of function 'memcpy' [-Wimplicit-function-declaration]
>       return memcpy(dest, src, n);
>              ^
>>> arch/x86/boot/compressed/string.c:56:10: warning: return makes pointer from integer without a cast [-Wint-conversion]
>    arch/x86/boot/compressed/string.c: At top level:
>>> arch/x86/boot/compressed/string.c:65:7: error: conflicting types for 'memcpy'
>     void *memcpy(void *dest, const void *src, size_t n)
>           ^
>    arch/x86/boot/compressed/string.c:56:10: note: previous implicit declaration of 'memcpy' was here
>       return memcpy(dest, src, n);
>              ^
>    arch/x86/boot/compressed/string.c: In function 'memcpy':
>>> arch/x86/boot/compressed/string.c:68:3: warning: implicit declaration of function 'warn' [-Wimplicit-function-declaration]
>       warn("Avoiding potentially unsafe overlapping memcpy()!");
>       ^
>
> vim +/memcpy +65 arch/x86/boot/compressed/string.c
>
>     50  void *memmove(void *dest, const void *src, size_t n)
>     51  {
>     52          unsigned char *d = dest;
>     53          const unsigned char *s = src;
>     54
>     55          if (d <= s || d - s >= n)
>   > 56                  return memcpy(dest, src, n);
>     57
>     58          while (n-- > 0)
>     59                  d[n] = s[n];
>     60
>     61          return dest;
>     62  }
>     63
>     64  /* Detect and warn about potential overlaps, but handle them with memmove. */
>   > 65  void *memcpy(void *dest, const void *src, size_t n)
>     66  {
>     67          if (dest > src && dest - src < n) {
>   > 68                  warn("Avoiding potentially unsafe overlapping memcpy()!");
>     69                  return memmove(dest, src, n);
>     70          }
>     71          return __memcpy(dest, src, n);
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation



-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web