[PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()

Kees Cook posted 1 patch 6 months, 3 weeks ago
arch/x86/include/asm/string_32.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()
Posted by Kees Cook 6 months, 3 weeks ago
Add basic sanity checking for pathological "size" arguments to
memcpy(). Besides the run-time checking benefit, this avoids
GCC trying to be very smart about value range tracking[1] when
CONFIG_PROFILE_ALL_BRANCHES=y but FORTIFY_SOURCE=n.

Link: https://lore.kernel.org/all/202505191117.C094A90F88@keescook/ [1]
Reported-by: Randy Dunlap <rdunlap@infradead.org>
Closes: https://lore.kernel.org/all/e3754f69-1dea-4542-8de0-a567a14fb95b@infradead.org/
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Kees Cook <kees@kernel.org>
---
 v2: isolate this specifically to 32-bit x86 -- doing this generally is much more work
 v1: https://lore.kernel.org/lkml/20250520163320.work.924-kees@kernel.org/
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: <x86@kernel.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
---
 arch/x86/include/asm/string_32.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/string_32.h b/arch/x86/include/asm/string_32.h
index 32c0d981a82a..6e8d100d1301 100644
--- a/arch/x86/include/asm/string_32.h
+++ b/arch/x86/include/asm/string_32.h
@@ -147,7 +147,14 @@ extern void *memcpy(void *, const void *, size_t);
 
 #ifndef CONFIG_FORTIFY_SOURCE
 
-#define memcpy(t, f, n) __builtin_memcpy(t, f, n)
+#define memcpy(t, f, n)					\
+	({						\
+		typeof(n) __n = (n);			\
+		/* Skip impossible sizes. */		\
+		if (!(__n < 0 || __n == SIZE_MAX))	\
+			__builtin_memcpy(t, f, __n);	\
+		(t);					\
+	})
 
 #endif /* !CONFIG_FORTIFY_SOURCE */
 
-- 
2.34.1
Re: [PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()
Posted by kernel test robot 6 months, 3 weeks ago
Hi Kees,

kernel test robot noticed the following build errors:

[auto build test ERROR on tip/master]
[also build test ERROR on tip/x86/core kees/for-next/pstore kees/for-next/kspp linus/master v6.15-rc7 next-20250523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/x86-string_32-h-Provide-basic-sanity-checks-for-fallback-memcpy/20250523-122803
base:   tip/master
patch link:    https://lore.kernel.org/r/20250523042635.work.579-kees%40kernel.org
patch subject: [PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()
config: i386-buildonly-randconfig-004-20250524 (https://download.01.org/0day-ci/archive/20250524/202505240850.vTrqyqJx-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505240850.vTrqyqJx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505240850.vTrqyqJx-lkp@intel.com/

All errors (new ones prefixed by >>):

   arch/x86/crypto/serpent_sse2_glue.c: In function 'serpent_decrypt_cbc_xway':
>> arch/x86/crypto/serpent_sse2_glue.c:39:19: error: assignment to 'const u8 *' {aka 'const unsigned char *'} from incompatible pointer type 'u8 (*)[16]' {aka 'unsigned char (*)[16]'} [-Werror=incompatible-pointer-types]
      39 |                 s = memcpy(buf, src, sizeof(buf));
         |                   ^
   cc1: some warnings being treated as errors


vim +39 arch/x86/crypto/serpent_sse2_glue.c

e0f409dcb82e463 Eric Biggers    2018-02-19  32  
9ad58b46f814edd Ard Biesheuvel  2021-01-05  33  static void serpent_decrypt_cbc_xway(const void *ctx, u8 *dst, const u8 *src)
e81792fbc2a6fa4 Jussi Kivilinna 2012-06-18  34  {
9ad58b46f814edd Ard Biesheuvel  2021-01-05  35  	u8 buf[SERPENT_PARALLEL_BLOCKS - 1][SERPENT_BLOCK_SIZE];
9ad58b46f814edd Ard Biesheuvel  2021-01-05  36  	const u8 *s = src;
e81792fbc2a6fa4 Jussi Kivilinna 2012-06-18  37  
9ad58b46f814edd Ard Biesheuvel  2021-01-05  38  	if (dst == src)
9ad58b46f814edd Ard Biesheuvel  2021-01-05 @39  		s = memcpy(buf, src, sizeof(buf));
9ad58b46f814edd Ard Biesheuvel  2021-01-05  40  	serpent_dec_blk_xway(ctx, dst, src);
9ad58b46f814edd Ard Biesheuvel  2021-01-05  41  	crypto_xor(dst + SERPENT_BLOCK_SIZE, s, sizeof(buf));
e81792fbc2a6fa4 Jussi Kivilinna 2012-06-18  42  }
e81792fbc2a6fa4 Jussi Kivilinna 2012-06-18  43  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()
Posted by kernel test robot 6 months, 3 weeks ago
Hi Kees,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/master]
[also build test WARNING on tip/x86/core kees/for-next/pstore kees/for-next/kspp linus/master v6.15-rc7 next-20250523]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Kees-Cook/x86-string_32-h-Provide-basic-sanity-checks-for-fallback-memcpy/20250523-122803
base:   tip/master
patch link:    https://lore.kernel.org/r/20250523042635.work.579-kees%40kernel.org
patch subject: [PATCH] x86: string_32.h: Provide basic sanity checks for fallback memcpy()
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20250524/202505240843.PwhJUSZt-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250524/202505240843.PwhJUSZt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505240843.PwhJUSZt-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> fs/ioctl.c:773:2: warning: result of comparison of constant 4294967295 with expression of type 'typeof (sb->s_uuid_len)' (aka 'unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
     773 |         memcpy(&u.uuid[0], &sb->s_uuid, sb->s_uuid_len);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
   1 warning generated.
--
   In file included from fs/select.c:33:
   In file included from include/net/busy_poll.h:18:
   In file included from include/net/ip.h:29:
   In file included from include/net/inet_sock.h:23:
   In file included from include/net/sock.h:66:
   In file included from include/net/dst.h:20:
>> include/net/neighbour.h:582:3: warning: result of comparison of constant 4294967295 with expression of type 'typeof (dev->addr_len)' (aka 'const unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
     582 |                 memcpy(dst, n->ha, dev->addr_len);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
   1 warning generated.
--
   In file included from init/do_mounts.c:23:
   In file included from include/linux/nfs_fs.h:32:
   In file included from include/linux/sunrpc/clnt.h:29:
   In file included from include/net/ipv6.h:12:
   In file included from include/linux/ipv6.h:102:
   In file included from include/linux/tcp.h:19:
   In file included from include/net/sock.h:66:
   In file included from include/net/dst.h:20:
>> include/net/neighbour.h:582:3: warning: result of comparison of constant 4294967295 with expression of type 'typeof (dev->addr_len)' (aka 'const unsigned char') is always false [-Wtautological-constant-out-of-range-compare]
     582 |                 memcpy(dst, n->ha, dev->addr_len);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
   In file included from init/do_mounts.c:23:
   In file included from include/linux/nfs_fs.h:38:
>> include/linux/nfs.h:46:2: warning: result of comparison of constant 4294967295 with expression of type 'typeof (source->size)' (aka 'const unsigned short') is always false [-Wtautological-constant-out-of-range-compare]
      46 |         memcpy(target->data, source->data, source->size);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
   2 warnings generated.
--
>> kernel/printk/printk.c:1144:2: warning: result of comparison of constant 4294967295 with expression of type 'typeof (r->info->text_len)' (aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare]
    1144 |         memcpy(&dest_r.text_buf[0], &r->text_buf[0], r->info->text_len);
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
>> kernel/printk/printk.c:2338:3: warning: result of comparison of constant 4294967295 with expression of type 'typeof (trunc_msg_len)' (aka 'unsigned short') is always false [-Wtautological-constant-out-of-range-compare]
    2338 |                 memcpy(&r.text_buf[text_len], trunc_msg, trunc_msg_len);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/x86/include/asm/string_32.h:154:24: note: expanded from macro 'memcpy'
     154 |                 if (!(__n < 0 || __n == SIZE_MAX))      \
         |                                  ~~~ ^  ~~~~~~~~
   2 warnings generated.


vim +773 fs/ioctl.c

4c5b47997521206 Miklos Szeredi  2021-04-07  764  
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  765  static int ioctl_getfsuuid(struct file *file, void __user *argp)
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  766  {
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  767  	struct super_block *sb = file_inode(file)->i_sb;
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  768  	struct fsuuid2 u = { .len = sb->s_uuid_len, };
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  769  
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  770  	if (!sb->s_uuid_len)
abe6acfa7d7b666 Günther Noack   2024-04-05  771  		return -ENOTTY;
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  772  
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06 @773  	memcpy(&u.uuid[0], &sb->s_uuid, sb->s_uuid_len);
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  774  
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  775  	return copy_to_user(argp, &u, sizeof(u)) ? -EFAULT : 0;
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  776  }
41bcbe59c3b3fa7 Kent Overstreet 2024-02-06  777  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki