linux-hardening - [PATCH] uaccess: Add minimum bounds check on kernel buffer size

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Fri,  3 Feb 2023 19:35:30 +0000
From:   Kees Cook <keescook@...omium.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Kees Cook <keescook@...omium.org>,
        Aleksa Sarai <cyphar@...har.com>,
        Christian Brauner <christian.brauner@...ntu.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        Dinh Nguyen <dinguyen@...nel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Alexander Potapenko <glider@...gle.com>,
        Christian Brauner <brauner@...nel.org>,
        Stafford Horne <shorne@...il.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

While there is logic about the difference between ksize and usize,
copy_struct_from_user() didn't check the size of the destination buffer
(when it was known) against ksize. Add this check so there is an upper
bounds check on the possible memset() call, otherwise lower bounds
checks made by callers will trigger bounds warnings under -Warray-bounds.
Seen under GCC 13:

In function 'copy_struct_from_user',
    inlined from 'iommufd_fops_ioctl' at
../drivers/iommu/iommufd/main.c:333:8:
../include/linux/fortify-string.h:59:33: warning: '__builtin_memset' offset [57, 4294967294] is out of the bounds [0, 56] of object 'buf' with type 'union ucmd_buffer' [-Warray-bounds=]
   59 | #define __underlying_memset     __builtin_memset
      |                                 ^
../include/linux/fortify-string.h:453:9: note: in expansion of macro '__underlying_memset'
  453 |         __underlying_memset(p, c, __fortify_size); \
      |         ^~~~~~~~~~~~~~~~~~~
../include/linux/fortify-string.h:461:25: note: in expansion of macro '__fortify_memset_chk'
  461 | #define memset(p, c, s) __fortify_memset_chk(p, c, s, \
      |                         ^~~~~~~~~~~~~~~~~~~~
../include/linux/uaccess.h:334:17: note: in expansion of macro 'memset'
  334 |                 memset(dst + size, 0, rest);
      |                 ^~~~~~
../drivers/iommu/iommufd/main.c: In function 'iommufd_fops_ioctl':
../drivers/iommu/iommufd/main.c:311:27: note: 'buf' declared here
  311 |         union ucmd_buffer buf;
      |                           ^~~

Cc: Aleksa Sarai <cyphar@...har.com>
Cc: Christian Brauner <christian.brauner@...ntu.com>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>
Cc: Arnd Bergmann <arnd@...db.de>
Cc: Dinh Nguyen <dinguyen@...nel.org>
Cc: Catalin Marinas <catalin.marinas@....com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Alexander Potapenko <glider@...gle.com>
Signed-off-by: Kees Cook <keescook@...omium.org>
---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index afb18f198843..ab9728138ad6 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -329,6 +329,10 @@ copy_struct_from_user(void *dst, size_t ksize, const void __user *src,
 	size_t size = min(ksize, usize);
 	size_t rest = max(ksize, usize) - size;
 
+	/* Double check if ksize is larger than a known object size. */
+	if (WARN_ON_ONCE(ksize > __builtin_object_size(dst, 1)))
+		return -E2BIG;
+
 	/* Deal with trailing bytes. */
 	if (usize < ksize) {
 		memset(dst + size, 0, rest);
-- 
2.34.1