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:   Fri,  2 Feb 2018 16:27:31 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Ben Skeggs <bskeggs@...hat.com>, David Airlie <airlied@...ux.ie>
Cc:     Arnd Bergmann <arnd@...db.de>, Martin Sebor <msebor@....gnu.org>,
        dri-devel@...ts.freedesktop.org, nouveau@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] drm: nouveau: use larger buffer in nvif_vmm_map

gcc points out a buffer that is clearly too small to be used
in a meaningful way, as the 'sizeof(*args) + argc > sizeof(stack)'
will always fail:

In function 'memcpy',
    inlined from 'nvif_vmm_map' at drivers/gpu/drm/nouveau/nvif/vmm.c:55:2:
include/linux/string.h:353:9: error: '__builtin_memcpy' offset 40 is out of the bounds [0, 16] of object 'stack' with type 'u8[16]' {aka 'unsigned char[16]'} [-Werror=array-bounds]
  return __builtin_memcpy(p, q, size);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/nouveau/nvif/vmm.c: In function 'nvif_vmm_map':
drivers/gpu/drm/nouveau/nvif/vmm.c:40:5: note: 'stack' declared here

This makes the buffer large enough so it should serve the purpose
that the author presumably had in mind. Alternatively we could
just get rid of it completely and simplify the code at the cost
of always doing the kmalloc (as we do in the current version).

Fixes: 920d2b5ef215 ("drm/nouveau/mmu: define user interfaces to mmu vmm opertaions")
Signed-off-by: Arnd Bergmann <arnd@...db.de>
---
Cc: Martin Sebor <msebor@....gnu.org>

Martin: this one is interesting, I think it qualifies as a
false-positive warning that gcc should not print because there
is no overflow, but the code is still wrong because we never
copy into the fixed-size buffer that was intended as a
micro-optimization
---
 drivers/gpu/drm/nouveau/nvif/vmm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvif/vmm.c b/drivers/gpu/drm/nouveau/nvif/vmm.c
index 31cdb2d2e1ff..191832be6c65 100644
--- a/drivers/gpu/drm/nouveau/nvif/vmm.c
+++ b/drivers/gpu/drm/nouveau/nvif/vmm.c
@@ -37,7 +37,7 @@ nvif_vmm_map(struct nvif_vmm *vmm, u64 addr, u64 size, void *argv, u32 argc,
 	     struct nvif_mem *mem, u64 offset)
 {
 	struct nvif_vmm_map_v0 *args;
-	u8 stack[16];
+	u8 stack[48];
 	int ret;
 
 	if (sizeof(*args) + argc > sizeof(stack)) {
-- 
2.9.0

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ