Nouveau Archive on lore.kernel.org
 help / color / Atom feed
* [Nouveau] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy()
@ 2021-02-25 11:38 Luo Jiaxing
  2021-02-26  1:01 ` [Nouveau] [Linuxarm] " Song Bao Hua (Barry Song)
  0 siblings, 1 reply; 3+ messages in thread
From: Luo Jiaxing @ 2021-02-25 11:38 UTC (permalink / raw)
  To: nouveau, dri-devel, bskeggs; +Cc: luojiaxing, linux-kernel, linuxarm

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;
 }
 
-- 
2.7.4

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Nouveau] [Linuxarm] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy()
  2021-02-25 11:38 [Nouveau] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy() Luo Jiaxing
@ 2021-02-26  1:01 ` Song Bao Hua (Barry Song)
  2021-02-27  9:20   ` luojiaxing
  0 siblings, 1 reply; 3+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-02-26  1:01 UTC (permalink / raw)
  To: luojiaxing, nouveau, dri-devel, bskeggs
  Cc: luojiaxing, linux-kernel, linuxarm



> -----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

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Nouveau] [Linuxarm] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy()
  2021-02-26  1:01 ` [Nouveau] [Linuxarm] " Song Bao Hua (Barry Song)
@ 2021-02-27  9:20   ` luojiaxing
  0 siblings, 0 replies; 3+ messages in thread
From: luojiaxing @ 2021-02-27  9:20 UTC (permalink / raw)
  To: Song Bao Hua (Barry Song), nouveau, dri-devel, bskeggs
  Cc: linux-kernel, linuxarm


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
>

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-25 11:38 [Nouveau] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy() Luo Jiaxing
2021-02-26  1:01 ` [Nouveau] [Linuxarm] " Song Bao Hua (Barry Song)
2021-02-27  9:20   ` luojiaxing

Nouveau Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/nouveau/0 nouveau/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 nouveau nouveau/ https://lore.kernel.org/nouveau \
		nouveau@lists.freedesktop.org
	public-inbox-index nouveau

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.freedesktop.lists.nouveau


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git