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

On 2020/8/8 20:50, David Laight wrote:

From: luobin (L)

quoted

Sent: 08 August 2020 04:37

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

quoted

From: Luo bin

quoted

Sent: 07 August 2020 03:09

fix the compile warnings of 'strncpy' output truncated before
terminating nul copying N bytes from a string of the same length

Signed-off-by: Luo bin <redacted>
Reported-by: kernel test robot <redacted>
---
V0~V1:
- use the strlen()+1 pattern consistently

 drivers/net/ethernet/huawei/hinic/hinic_devlink.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

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?

	David

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?

If you can't see it you probably shouldn't be submitting patches....

Consider what happens when the string is long.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Thanks for your explanation and review. The fault_level[level] is a fixed and NUL-terminated character
string in that code and its length is smaller than the dest buffer size so I think using
strlen(fault_level[level]) + 1 will not overflow the destination buffer. But using strncpy() on
NUL-terminated strings is dangerous indeed and there is totally no need to use it in that code
as Kees points out.