(adding linux-hardening@xxxxxxxxxxxxxxx) Le 18/05/2024 à 16:37, Guenter Roeck a écrit :
Trying to build parisc:allmodconfig with gcc 12.x or later results in the following build error. drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_mthd': drivers/gpu/drm/nouveau/nvif/object.c:161:9: error: 'memcpy' accessing 4294967264 or more bytes at offsets 0 and 32 overlaps 6442450881 bytes at offset -2147483617 [-Werror=restrict] 161 | memcpy(data, args->mthd.data, size); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/nouveau/nvif/object.c: In function 'nvif_object_ctor': drivers/gpu/drm/nouveau/nvif/object.c:298:17: error: 'memcpy' accessing 4294967240 or more bytes at offsets 0 and 56 overlaps 6442450833 bytes at offset -2147483593 [-Werror=restrict] 298 | memcpy(data, args->new.data, size); gcc assumes that 'sizeof(*args) + size' can overflow, which would result in the problem. The problem is not new, only it is now no longer a warning but an error since W=1 has been enabled for the drm subsystem and since Werror is enabled for test builds. Rearrange arithmetic and add extra size checks to avoid the overflow. Fixes: a61ddb4393ad ("drm: enable (most) W=1 warnings by default across the subsystem") Cc: Javier Martinez Canillas <javierm-H+wXaHxf7aLQT0dZR+AlfA@xxxxxxxxxxxxxxxx> Cc: Jani Nikula <jani.nikula-ral2JQCrhuEAvxtiuMwx3w@xxxxxxxxxxxxxxxx> Cc: Thomas Zimmermann <tzimmermann-l3A5Bk7waGM@xxxxxxxxxxxxxxxx> Cc: Danilo Krummrich <dakr-H+wXaHxf7aLQT0dZR+AlfA@xxxxxxxxxxxxxxxx> Cc: Maxime Ripard <mripard-DgEjT+Ai2ygdnm+yROfE0A@xxxxxxxxxxxxxxxx> Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@xxxxxxxxxxxxxxxx> --- checkpatch complains about the line length in the description and the (pre-existing) assignlemts in if conditions, but I did not want to split lines in the description or rearrange the code further. I don't know why I only see the problem with parisc builds (at least so far). drivers/gpu/drm/nouveau/nvif/object.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nvif/object.c b/drivers/gpu/drm/nouveau/nvif/object.c index 4d1aaee8fe15..baf623a48874 100644 --- a/drivers/gpu/drm/nouveau/nvif/object.c +++ b/drivers/gpu/drm/nouveau/nvif/object.c @@ -145,8 +145,9 @@ nvif_object_mthd(struct nvif_object *object, u32 mthd, void *data, u32 size) u8 stack[128]; int ret;- if (sizeof(*args) + size > sizeof(stack)) {- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) + if (size > sizeof(stack) - sizeof(*args)) { + if (size > INT_MAX || + !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL)))
Hi, Would it be cleaner or better to use size_add(sizeof(*args), size)?
return -ENOMEM; } else { args = (void *)stack; @@ -276,7 +277,8 @@ nvif_object_ctor(struct nvif_object *parent, const char *name, u32 handle, object->map.size = 0;if (parent) {- if (!(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) { + if (size > INT_MAX || + !(args = kmalloc(sizeof(*args) + size, GFP_KERNEL))) {
Same. CJ
nvif_object_dtor(object); return -ENOMEM; }
