| Message ID | 20210611160529.88936-1-lijunp213@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] ibmvnic: fix kernel build warning in strncpy | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 9 maintainers not CCed: tlfalcon@linux.ibm.com drt@linux.ibm.com paulus@samba.org sukadev@linux.ibm.com benh@kernel.crashing.org linuxppc-dev@lists.ozlabs.org mpe@ellerman.id.au davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 3 this patch: 3 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On 6/11/21 9:05 AM, Lijun Pan wrote: > drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’: > drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] > 4393 | strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Signed-off-by: Lijun Pan <lijunp213@gmail.com> > --- > drivers/net/ethernet/ibm/ibmvnic.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c > index 497f1a7da70b..2675b2301ed7 100644 > --- a/drivers/net/ethernet/ibm/ibmvnic.c > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > @@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq, > > complete: > if (adapter->fw_version[0] == '\0') > - strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > + memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > complete(&adapter->fw_done); > } > This doesn't fix the real problem. The error message is saying that there is no string terminating '\0' byte getting set after the "N/A" string, meaning that there could be garbage in the buffer after the string that could allow for surprising and bad things to happen when that string is used later, including buffer overruns that can cause stack smash or other memory munging. Better would be to use strlcpy() with a limiter of sizeof(adapter->fw_version). sln
On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson <snelson@pensando.io> wrote: > > On 6/11/21 9:05 AM, Lijun Pan wrote: > > drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’: > > drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] > > 4393 | strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > > Signed-off-by: Lijun Pan <lijunp213@gmail.com> > > --- > > drivers/net/ethernet/ibm/ibmvnic.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c > > index 497f1a7da70b..2675b2301ed7 100644 > > --- a/drivers/net/ethernet/ibm/ibmvnic.c > > +++ b/drivers/net/ethernet/ibm/ibmvnic.c > > @@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq, > > > > complete: > > if (adapter->fw_version[0] == '\0') > > - strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > > + memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); > > complete(&adapter->fw_done); > > } > > > > This doesn't fix the real problem. The error message is saying that > there is no string terminating '\0' byte getting set after the "N/A" > string, meaning that there could be garbage in the buffer after the > string that could allow for surprising and bad things to happen when > that string is used later, including buffer overruns that can cause > stack smash or other memory munging. > > Better would be to use strlcpy() with a limiter of > sizeof(adapter->fw_version). > > sln Thanks for the tip. I looked up both strscpy and strlcpy. It seems nowadays strscpy is preferred.
On 6/11/21 11:36 AM, Lijun Pan wrote: > On Fri, Jun 11, 2021 at 11:28 AM Shannon Nelson <snelson@pensando.io> wrote: >> On 6/11/21 9:05 AM, Lijun Pan wrote: >>> drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’: >>> drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] >>> 4393 | strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> >>> Signed-off-by: Lijun Pan <lijunp213@gmail.com> >>> --- >>> drivers/net/ethernet/ibm/ibmvnic.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c >>> index 497f1a7da70b..2675b2301ed7 100644 >>> --- a/drivers/net/ethernet/ibm/ibmvnic.c >>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c >>> @@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq, >>> >>> complete: >>> if (adapter->fw_version[0] == '\0') >>> - strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); >>> + memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); >>> complete(&adapter->fw_done); >>> } >>> >> This doesn't fix the real problem. The error message is saying that >> there is no string terminating '\0' byte getting set after the "N/A" >> string, meaning that there could be garbage in the buffer after the >> string that could allow for surprising and bad things to happen when >> that string is used later, including buffer overruns that can cause >> stack smash or other memory munging. >> >> Better would be to use strlcpy() with a limiter of >> sizeof(adapter->fw_version). >> >> sln > Thanks for the tip. I looked up both strscpy and strlcpy. It seems nowadays > strscpy is preferred. Sure, that works too. sln
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c index 497f1a7da70b..2675b2301ed7 100644 --- a/drivers/net/ethernet/ibm/ibmvnic.c +++ b/drivers/net/ethernet/ibm/ibmvnic.c @@ -4390,7 +4390,7 @@ static void handle_vpd_rsp(union ibmvnic_crq *crq, complete: if (adapter->fw_version[0] == '\0') - strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); + memcpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); complete(&adapter->fw_done); }
drivers/net/ethernet/ibm/ibmvnic.c: In function ‘handle_vpd_rsp’: drivers/net/ethernet/ibm/ibmvnic.c:4393:3: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] 4393 | strncpy((char *)adapter->fw_version, "N/A", 3 * sizeof(char)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Signed-off-by: Lijun Pan <lijunp213@gmail.com> --- drivers/net/ethernet/ibm/ibmvnic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)