From: luojiaxing <luojiaxing@huawei.com>
To: "Song Bao Hua (Barry Song)" <song.bao.hua@hisilicon.com>,
"nouveau@lists.freedesktop.org" <nouveau@lists.freedesktop.org>,
"dri-devel@lists.freedesktop.org"
<dri-devel@lists.freedesktop.org>,
"bskeggs@redhat.com" <bskeggs@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linuxarm@openeuler.org" <linuxarm@openeuler.org>
Subject: Re: [Nouveau] [Linuxarm] [PATCH v1] drm/nouveau/device: append a NUL-terminated character for the string which filled by strncpy()
Date: Sat, 27 Feb 2021 17:20:24 +0800 [thread overview]
Message-ID: <27a28507-f394-43af-998e-6d03a971b85d@huawei.com> (raw)
In-Reply-To: <1b841f487ad742ee941282b534bdcb4d@hisilicon.com>
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
prev parent reply other threads:[~2021-02-28 23:47 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27a28507-f394-43af-998e-6d03a971b85d@huawei.com \
--to=luojiaxing@huawei.com \
--cc=bskeggs@redhat.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxarm@openeuler.org \
--cc=nouveau@lists.freedesktop.org \
--cc=song.bao.hua@hisilicon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).