Fix the following warning.
tools/testing/selftests/net/ipsec.c:230:25: warning: Possible null pointer dereference: payload [nullPointer] memcpy(RTA_DATA(attr), payload, size); ^ tools/testing/selftests/net/ipsec.c:1618:54: note: Calling function 'rtattr_pack', 4th argument 'NULL' value is 0 if (rtattr_pack(&req.nh, sizeof(req), XFRMA_IF_ID, NULL, 0)) { ^ tools/testing/selftests/net/ipsec.c:230:25: note: Null pointer dereference memcpy(RTA_DATA(attr), payload, size); ^
Signed-off-by: liuye liuye@kylinos.cn --- tools/testing/selftests/net/ipsec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/ipsec.c b/tools/testing/selftests/net/ipsec.c index be4a30a0d02a..725310ac26a9 100644 --- a/tools/testing/selftests/net/ipsec.c +++ b/tools/testing/selftests/net/ipsec.c @@ -227,7 +227,8 @@ static int rtattr_pack(struct nlmsghdr *nh, size_t req_sz,
attr->rta_len = RTA_LENGTH(size); attr->rta_type = rta_type; - memcpy(RTA_DATA(attr), payload, size); + if (payload != NULL) + memcpy(RTA_DATA(attr), payload, size);
return 0; }
On Tue, Jan 14, 2025 at 03:43:29PM +0800, liuye wrote:
Fix the following warning.
I think it is a bit more than a warning, I'd phrase this more like, even as it repeats the subject. Also, it would be nice to cite the tool that generates the warning.
Address Null pointer dereference in rtattr_pack.
Flagged by ??? as:
tools/testing/selftests/net/ipsec.c:230:25: warning: Possible null pointer dereference: payload [nullPointer] memcpy(RTA_DATA(attr), payload, size); ^ tools/testing/selftests/net/ipsec.c:1618:54: note: Calling function 'rtattr_pack', 4th argument 'NULL' value is 0 if (rtattr_pack(&req.nh, sizeof(req), XFRMA_IF_ID, NULL, 0)) { ^ tools/testing/selftests/net/ipsec.c:230:25: note: Null pointer dereference memcpy(RTA_DATA(attr), payload, size); ^
And I wonder if a fixes tag is appropriate, and if so this one:
70bfdf62e93a ("selftests/net/ipsec: Add test for xfrm_spdattr_type_t")
And, accordingly if this patch should be targeted at net:
[PATCH net] ...
Signed-off-by: liuye liuye@kylinos.cn
Please consider separating out your family and given name in hte signed-off-by line. Perhaps Lin Ye (apologies if that is incorrect).
The above not withstanding, the code change looks good to me. So feel free to include the following in a v2 with an updated patch description.
tools/testing/selftests/net/ipsec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/ipsec.c b/tools/testing/selftests/net/ipsec.c index be4a30a0d02a..725310ac26a9 100644 --- a/tools/testing/selftests/net/ipsec.c +++ b/tools/testing/selftests/net/ipsec.c @@ -227,7 +227,8 @@ static int rtattr_pack(struct nlmsghdr *nh, size_t req_sz, attr->rta_len = RTA_LENGTH(size); attr->rta_type = rta_type;
- memcpy(RTA_DATA(attr), payload, size);
- if (payload != NULL)
I think it would be more idiomatic to express this as:
if (payload)
memcpy(RTA_DATA(attr), payload, size);
return 0; } -- 2.25.1
From: liuye liuye@kylinos.cn
From: Liu Ye liuye@kylinos.cn
Address Null pointer dereference in rtattr_pack.
Flagged by cppcheck as: tools/testing/selftests/net/ipsec.c:230:25: warning: Possible null pointer dereference: payload [nullPointer] memcpy(RTA_DATA(attr), payload, size); ^ tools/testing/selftests/net/ipsec.c:1618:54: note: Calling function 'rtattr_pack', 4th argument 'NULL' value is 0 if (rtattr_pack(&req.nh, sizeof(req), XFRMA_IF_ID, NULL, 0)) { ^ tools/testing/selftests/net/ipsec.c:230:25: note: Null pointer dereference memcpy(RTA_DATA(attr), payload, size); ^ Fixes: 70bfdf62e93a ("selftests/net/ipsec: Add test for xfrm_spdattr_type_t") --- V2: Modify description. Add code checking tools. Separating family and given name in Signed-off-by line. Modify code format. Add fixes. ---
Signed-off-by: Liu Ye liuye@kylinos.cn --- tools/testing/selftests/net/ipsec.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/net/ipsec.c b/tools/testing/selftests/net/ipsec.c index be4a30a0d02a..9b44a091802c 100644 --- a/tools/testing/selftests/net/ipsec.c +++ b/tools/testing/selftests/net/ipsec.c @@ -227,7 +227,8 @@ static int rtattr_pack(struct nlmsghdr *nh, size_t req_sz,
attr->rta_len = RTA_LENGTH(size); attr->rta_type = rta_type; - memcpy(RTA_DATA(attr), payload, size); + if (payload) + memcpy(RTA_DATA(attr), payload, size);
return 0; }
On Wed, 15 Jan 2025 11:13:22 +0800 Liu Ye wrote:
From: liuye liuye@kylinos.cn
From: Liu Ye liuye@kylinos.cn
too many From lines.
Address Null pointer dereference in rtattr_pack.
I think size is 0 in the bad case, so it's more of an undefinied behavior.
Flagged by cppcheck as: tools/testing/selftests/net/ipsec.c:230:25: warning: Possible null pointer dereference: payload [nullPointer] memcpy(RTA_DATA(attr), payload, size); ^ tools/testing/selftests/net/ipsec.c:1618:54: note: Calling function 'rtattr_pack', 4th argument 'NULL' value is 0 if (rtattr_pack(&req.nh, sizeof(req), XFRMA_IF_ID, NULL, 0)) { ^ tools/testing/selftests/net/ipsec.c:230:25: note: Null pointer dereference memcpy(RTA_DATA(attr), payload, size); ^ Fixes: 70bfdf62e93a ("selftests/net/ipsec: Add test for xfrm_spdattr_type_t")
Your Sign-off needs to be right after fixes.
V2: Modify description. Add code checking tools. Separating family and given name in Signed-off-by line. Modify code format. Add fixes.
Please post v3 as a new thread (not in reply to).
linux-kselftest-mirror@lists.linaro.org