linux-hardening - [PATCH v3] tpm: vtpm_proxy: Check length to avoid compiler warning

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>] [day] [month] [year] [list]

Date:   Wed, 19 Jan 2022 10:43:54 -0800
From:   Kees Cook <keescook@...omium.org>
To:     Peter Huewe <peterhuewe@....de>
Cc:     Kees Cook <keescook@...omium.org>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Jason Gunthorpe <jgg@...pe.ca>,
        linux-integrity@...r.kernel.org,
        Stefan Berger <stefanb@...ux.ibm.com>,
        Jann Horn <jannh@...gle.com>, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org
Subject: [PATCH v3] tpm: vtpm_proxy: Check length to avoid compiler warning

When building with -Warray-bounds under GCC 11.2, this warning was
emitted:

In function 'memset',
    inlined from 'vtpm_proxy_fops_read' at drivers/char/tpm/tpm_vtpm_proxy.c:102:2:
./include/linux/fortify-string.h:43:33: warning: '__builtin_memset' pointer overflow between offset 164 and size [2147483648, 4294967295]
[-Warray-bounds]
   43 | #define __underlying_memset     __builtin_memset
      |                                 ^

This warning appears to be triggered due to the "count < len"
check in vtpm_proxy_fops_read() splitting the CFG[1], and the compiler
attempting to reason about the possible value range in len compared
to the buffer size.

In order to silence this warning, and to keep this code robust if the
use of proxy_dev->req_len ever changes in the future, explicitly check
the size of len before reaching the memset().

[1] https://lore.kernel.org/lkml/CAG48ez1iTF9KegKJrW5a3WzXgCPZJ73nS2_e5esKJRppdzvv8g@mail.gmail.com

Cc: Peter Huewe <peterhuewe@....de>
Cc: Jarkko Sakkinen <jarkko@...nel.org>
Cc: Jason Gunthorpe <jgg@...pe.ca>
Cc: linux-integrity@...r.kernel.org
Reviewed-by: Stefan Berger <stefanb@...ux.ibm.com>
Link: https://lore.kernel.org/lkml/4b59d305-6858-1514-751a-37853ad777be@linux.ibm.com
Signed-off-by: Kees Cook <keescook@...omium.org>
---
v1: https://lore.kernel.org/lkml/20220113002727.3709495-1-keescook@chromium.org
v2: https://lore.kernel.org/lkml/20220118183650.3386989-1-keescook@chromium.org
v3: Add more details to the commit log, including a link to Jann's analysis
---
 drivers/char/tpm/tpm_vtpm_proxy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c
index 91c772e38bb5..5c865987ba5c 100644
--- a/drivers/char/tpm/tpm_vtpm_proxy.c
+++ b/drivers/char/tpm/tpm_vtpm_proxy.c
@@ -91,7 +91,7 @@ static ssize_t vtpm_proxy_fops_read(struct file *filp, char __user *buf,
 
 	len = proxy_dev->req_len;
 
-	if (count < len) {
+	if (count < len || len > sizeof(proxy_dev->buffer)) {
 		mutex_unlock(&proxy_dev->buf_lock);
 		pr_debug("Invalid size in recv: count=%zd, req_len=%zd\n",
 			 count, len);
-- 
2.30.2