On 2018/6/30 5:51, Andy Shevchenko wrote: > On Sat, Jun 30, 2018 at 12:47 AM, Andy Shevchenko > <andy.shevchenko@xxxxxxxxx> wrote: >> On Fri, Jun 29, 2018 at 5:51 AM, YueHaibing <yuehaibing@xxxxxxxxxx> wrote: >>> 'firmware' is a module param which may been longer than firmware_id, >>> so using strlcpy() to guard against overflows >> >> strncat() is against overflow, this does a bit more. >> >>> priv->firmware_id[0] = '\0'; >> ... >>> if (firmware) /* module parameter */ >>> - strcpy(priv->firmware_id, firmware); >>> + strlcpy(priv->firmware_id, firmware, sizeof(priv->firmware_id)); >> >> In either case the above '\0' is not needed. >> But it looks like the intention was to use strncat() / strlcat(). > > Ah, this is under condition, yes. If no parameter supplied, this needs > to be clean, but > priv is allocated with zeroed memory > https://elixir.bootlin.com/linux/latest/source/net/core/dev.c#L8369 so just remove this line: priv->firmware_id[0] = '\0'; and using strlcpy while 'firmware' NOT null is enough. Will send v2. >