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@xxxxxxxxxx>
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
>
