selftests: net: ipmr: Avoid memcpy() from NULL in nl_add_rtattr()

[PATCH] selftests: net: ipmr: Avoid memcpy() from NULL in nl_add_rtattr()

Posted by Chaithanya Lagisetty 3 weeks, 4 days ago

nl_add_rtattr() unconditionally does memcpy(RTA_DATA(rta), data, len).
For zero-length attributes the callers pass data == NULL and len == 0,
for example the RTA_PREFSRC attribute added for proxy MFC entries:

	if (mfc_attr->proxy)
		rta = nl_add_rtattr(nlmsg, rta, RTA_PREFSRC, NULL, 0);

Passing a NULL pointer to memcpy() is undefined behaviour even when the
length is zero, because its source parameter is marked
__attribute__((nonnull)); it is flagged by fortify/-Wnonnull.

Only call memcpy() when len is non-zero.

Fixes: 05068eaa67b2 ("selftest: net: Add basic functionality tests for ipmr.")
Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
---
 tools/testing/selftests/net/forwarding/ipmr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/forwarding/ipmr.c b/tools/testing/selftests/net/forwarding/ipmr.c
index 9cd9f70de132..d3e26341821c 100644
--- a/tools/testing/selftests/net/forwarding/ipmr.c
+++ b/tools/testing/selftests/net/forwarding/ipmr.c
@@ -120,7 +120,8 @@ static struct rtattr *nl_add_rtattr(struct nlmsghdr *nlmsg, struct rtattr *rta,
 
 	rta->rta_type = type;
 	rta->rta_len = RTA_LENGTH(len);
-	memcpy(RTA_DATA(rta), data, len);
+	if (len)
+		memcpy(RTA_DATA(rta), data, len);
 
 	nlmsg->nlmsg_len += NLMSG_ALIGN(rta->rta_len);
 
-- 
2.43.0

Re: [PATCH] selftests: net: ipmr: Avoid memcpy() from NULL in nl_add_rtattr()

Posted by Kuniyuki Iwashima 3 weeks, 3 days ago

On Tue, Sep 1, 2026 at 12:14 AM Chaithanya Lagisetty
<nagachaithanya9911@gmail.com> wrote:
>
> nl_add_rtattr() unconditionally does memcpy(RTA_DATA(rta), data, len).
> For zero-length attributes the callers pass data == NULL and len == 0,
> for example the RTA_PREFSRC attribute added for proxy MFC entries:
>
>         if (mfc_attr->proxy)
>                 rta = nl_add_rtattr(nlmsg, rta, RTA_PREFSRC, NULL, 0);
>
> Passing a NULL pointer to memcpy() is undefined behaviour even when the
> length is zero, because its source parameter is marked
> __attribute__((nonnull)); it is flagged by fortify/-Wnonnull.

It's not flagged since NULL is passed via nl_add_rtattr(), not directly
to memcpy().

Also, the behaviour will be well-defined with N3322 in C2y.

Given there is no real harm and we do not bother revisiting this,
there is no need to change that.

Re: [PATCH] selftests: net: ipmr: Avoid memcpy() from NULL in nl_add_rtattr()

Posted by Chaithanya Lagisetty 3 weeks, 3 days ago

On Tue, Sep 01, 2026 at 11:49:46PM +0000, Kuniyuki Iwashima wrote:
> It's not flagged since NULL is passed via nl_add_rtattr(), not directly
> to memcpy().
>
> Also, the behaviour will be well-defined with N3322 in C2y.
>
> Given there is no real harm and we do not bother revisiting this,
> there is no need to change that.

You are right, and thanks for the correction. The NULL is laundered
through the function parameter, so it is not diagnosed at the call site
the way my changelog claimed. Together with N3322 making this
well-defined, I agree there is nothing worth changing here.

Dropping the patch.

Hangbin, thanks for the review as well; sorry for the churn.

Thanks,
Chaithanya

Re: [PATCH] selftests: net: ipmr: Avoid memcpy() from NULL in nl_add_rtattr()

Posted by Hangbin Liu 3 weeks, 4 days ago

On Tue, Sep 01, 2026 at 07:13:53AM +0000, Chaithanya Lagisetty wrote:
> nl_add_rtattr() unconditionally does memcpy(RTA_DATA(rta), data, len).
> For zero-length attributes the callers pass data == NULL and len == 0,
> for example the RTA_PREFSRC attribute added for proxy MFC entries:
> 
> 	if (mfc_attr->proxy)
> 		rta = nl_add_rtattr(nlmsg, rta, RTA_PREFSRC, NULL, 0);
> 
> Passing a NULL pointer to memcpy() is undefined behaviour even when the
> length is zero, because its source parameter is marked
> __attribute__((nonnull)); it is flagged by fortify/-Wnonnull.
> 
> Only call memcpy() when len is non-zero.
> 
> Fixes: 05068eaa67b2 ("selftest: net: Add basic functionality tests for ipmr.")
> Signed-off-by: Chaithanya Lagisetty <nagachaithanya9911@gmail.com>
> ---
>  tools/testing/selftests/net/forwarding/ipmr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/forwarding/ipmr.c b/tools/testing/selftests/net/forwarding/ipmr.c
> index 9cd9f70de132..d3e26341821c 100644
> --- a/tools/testing/selftests/net/forwarding/ipmr.c
> +++ b/tools/testing/selftests/net/forwarding/ipmr.c
> @@ -120,7 +120,8 @@ static struct rtattr *nl_add_rtattr(struct nlmsghdr *nlmsg, struct rtattr *rta,
>  
>  	rta->rta_type = type;
>  	rta->rta_len = RTA_LENGTH(len);
> -	memcpy(RTA_DATA(rta), data, len);
> +	if (len)
> +		memcpy(RTA_DATA(rta), data, len);
>  
>  	nlmsg->nlmsg_len += NLMSG_ALIGN(rta->rta_len);
>  
> -- 
> 2.43.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>