RE: [Linuxarm] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy()
From: Song Bao Hua (Barry Song)
Date: Thu Feb 25 2021 - 20:02:20 EST
>
-----Original Message-----
>
From: Luo Jiaxing [mailto:luojiaxing@xxxxxxxxxx]
>
Sent: Friday, February 26, 2021 12:39 AM
>
To: nouveau@xxxxxxxxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx;
>
bskeggs@xxxxxxxxxx
>
Cc: linux-kernel@xxxxxxxxxxxxxxx; linuxarm@xxxxxxxxxxxxx; luojiaxing
>
<luojiaxing@xxxxxxxxxx>
>
Subject: [Linuxarm] [PATCH v1] drm/nouveau/device: append a NUL-terminated
>
character for the string which filled by strncpy()
>
>
Following warning is found when using W=1 to build kernel:
>
>
In function ‘nvkm_udevice_info’,
>
inlined from ‘nvkm_udevice_mthd’ at
>
drivers/gpu/drm/nouveau/nvkm/engine/device/user.c:195:10:
>
drivers/gpu/drm/nouveau/nvkm/engine/device/user.c:164:2: warning: ‘strncpy’
>
specified bound 16 equals destination size [-Wstringop-truncation]
>
164 | strncpy(args->v0.chip, device->chip->name, sizeof(args->v0.chip));
>
drivers/gpu/drm/nouveau/nvkm/engine/device/user.c:165:2: warning: ‘strncpy’
>
specified bound 64 equals destination size [-Wstringop-truncation]
>
165 | strncpy(args->v0.name, device->name, sizeof(args->v0.name));
>
>
The reason of this warning is strncpy() does not guarantee that the
>
destination buffer will be NUL terminated. If the length of source string
>
is bigger than number we set by third input parameter, only first [number]
>
of characters is copied to the destination, and no NUL-terminated is
>
automatically added. There are some potential risks.
>
>
Signed-off-by: Luo Jiaxing <luojiaxing@xxxxxxxxxx>
>
---
>
drivers/gpu/drm/nouveau/nvkm/engine/device/user.c | 6 ++++--
>
1 file changed, 4 insertions(+), 2 deletions(-)
>
>
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
>
b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
>
index fea9d8f..2a32fe0 100644
>
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
>
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c
>
@@ -161,8 +161,10 @@ nvkm_udevice_info(struct nvkm_udevice *udev, void *data,
>
u32 size)
>
if (imem && args->v0.ram_size > 0)
>
args->v0.ram_user = args->v0.ram_user - imem->reserved;
>
>
- strncpy(args->v0.chip, device->chip->name, sizeof(args->v0.chip));
>
- strncpy(args->v0.name, device->name, sizeof(args->v0.name));
>
+ strncpy(args->v0.chip, device->chip->name, sizeof(args->v0.chip) - 1);
>
+ args->v0.chip[sizeof(args->v0.chip) - 1] = '0円';
>
+ strncpy(args->v0.name, device->name, sizeof(args->v0.name) - 1);
>
+ args->v0.name[sizeof(args->v0.name) - 1] = '0円';
Isn't it better to use snprintf()?
>
return 0;
>
}
>
Thanks
Barry