Re: [PATCH net-next v1] hinic: fix strncpy output truncated compile warnings

On 2020/8/8 11:42, David Miller wrote:

From: "luobin (L)" <redacted>
Date: Sat, 8 Aug 2020 11:36:42 +0800

quoted

On 2020/8/7 17:32, David Laight wrote:

quoted

quoted

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c

b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
index c6adc776f3c8..1ec88ebf81d6 100644

--- a/drivers/net/ethernet/huawei/hinic/hinic_devlink.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_devlink.c

@@ -342,9 +342,9 @@ static int chip_fault_show(struct devlink_fmsg *fmsg,

 	level = event->event.chip.err_level;
 	if (level < FAULT_LEVEL_MAX)
-		strncpy(level_str, fault_level[level], strlen(fault_level[level]));
+		strncpy(level_str, fault_level[level], strlen(fault_level[level]) + 1);

Have you even considered what that code is actually doing?

 ...

quoted

I'm sorry that I haven't got what you mean and I haven't found any defects in that code. Can you explain more to me?

David is trying to express the same thing I was trying to explain to
you, you should use sizeof(level_str) as the third argument because
the code is trying to make sure that the destination buffer is not
overrun.

If you use the strlen() of the source buffer, the strncpy() can still
overflow the destination buffer.

Now do you understand?
.

Thanks for your explanation. I explained that why I didn't use sizeof(level_str) as the third argument in my previous reply e-mail to you.
Because using sizeof(level_str) as the third argument will still cause the following compile warning:

In function ‘strncpy’,
    inlined from ‘chip_fault_show’ at drivers/net/ethernet/huawei/hinic/hinic_devlink.c:345:3:
./include/linux/string.h:297:30: warning: ‘__builtin_strncpy’ specified bound 17 equals destination size [-Wstringop-truncation]
  297 | #define __underlying_strncpy __builtin_strncpy

Now I know that using strncpy() on NUL-terminated strings is deprecated as Kees Cook points out and actually there is no need to use it
in my code.