[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@huawei.com --- 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'; return 0; }
-----Original Message----- From: Luo Jiaxing [mailto:luojiaxing@huawei.com] Sent: Friday, February 26, 2021 12:39 AM To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; bskeggs@redhat.com Cc: linux-kernel@vger.kernel.org; linuxarm@openeuler.org; luojiaxing luojiaxing@huawei.com 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@huawei.com
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
On 2021/2/26 9:01, Song Bao Hua (Barry Song) wrote:
-----Original Message----- From: Luo Jiaxing [mailto:luojiaxing@huawei.com] Sent: Friday, February 26, 2021 12:39 AM To: nouveau@lists.freedesktop.org; dri-devel@lists.freedesktop.org; bskeggs@redhat.com Cc: linux-kernel@vger.kernel.org; linuxarm@openeuler.org; luojiaxing luojiaxing@huawei.com 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@huawei.com
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()?
yes, you are right, snprintf() is better. Most of drivers use snprintf() to format a string,
but still some examples in kernel that use it for copy.
I modify to code to the follow and I think it's the same with strncpy but more safety
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c index fea9d8f..4bf65bb 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/user.c @@ -161,8 +161,8 @@ 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)); + snprintf(args->v0.chip, sizeof(args->v0.chip), "%s", device->chip->name); + snprintf(args->v0.name, sizeof(args->v0.name), "%s", device->name);
Thanks
Jiaxing
return 0; }
Thanks Barry
dri-devel@lists.freedesktop.org
-
Luo Jiaxing -
luojiaxing
-
Song Bao Hua (Barry Song)