Re: Endless loop in netlink_msg_to_ifaddr

[Timo Teras <timo.teras@iki.fi>]

On Wed, 30 May 2018 11:57:03 +0200
Matej Kupljen <matej.kupljen@gmail.com> wrote:

> I am using OpenWRT device with MUSL C library version 1.1.19 and I am
> running custom binary on it. I noticed that during testing my program
> started using 99% CPU.
> I build OpenWRT myself so I have all the sources. I attached the
> gdbserver and checked what is going on.

Thanks for the report!

> As you can see the first message in netlink reply has a rta_len set
> to zero so the list is never traversed, only the first message is
> received every time.
> 
> I am not sure if this is the correct response from netlink, however
> the program is stucked here.
>
> Any ideas?
> Please CC me in reply.

That is invalid message to my understanding. Perhaps there's some new
extensions that allow it. Upstream (linux kernel) RTA_OK does do
additional checks against this situation.

The same issue probably affects if_nameindex.

I think the following should fix it:

diff --git a/src/network/netlink.h b/src/network/netlink.h
index 20700ac5..00dc7172 100644
--- a/src/network/netlink.h
+++ b/src/network/netlink.h
@@ -80,13 +80,13 @@ struct ifaddrmsg {
 #define NLMSG_DATALEN(nlh)	((nlh)->nlmsg_len-sizeof(struct nlmsghdr))
 #define NLMSG_DATAEND(nlh)	((char*)(nlh)+(nlh)->nlmsg_len)
 #define NLMSG_NEXT(nlh)		(struct nlmsghdr*)((char*)(nlh)+NETLINK_ALIGN((nlh)->nlmsg_len))
-#define NLMSG_OK(nlh,end)	((char*)(end)-(char*)(nlh) >= sizeof(struct nlmsghdr))
+#define NLMSG_OK(nlh,end)	((char*)(end)-(char*)(nlh) >= sizeof(struct nlmsghdr) && (nlh)->nlmsg_len >= sizeof(struct nlmsghdr))
 
 #define RTA_DATA(rta)		((void*)((char*)(rta)+sizeof(struct rtattr)))
 #define RTA_DATALEN(rta)	((rta)->rta_len-sizeof(struct rtattr))
 #define RTA_DATAEND(rta)	((char*)(rta)+(rta)->rta_len)
 #define RTA_NEXT(rta)		(struct rtattr*)((char*)(rta)+NETLINK_ALIGN((rta)->rta_len))
-#define RTA_OK(nlh,end)		((char*)(end)-(char*)(rta) >= sizeof(struct rtattr))
+#define RTA_OK(rta,end)		((char*)(end)-(char*)(rta) >= sizeof(struct rtattr) && (rta)->rta_len >= sizeof(struct rtattr))
 
 #define NLMSG_RTA(nlh,len)	((void*)((char*)(nlh)+sizeof(struct nlmsghdr)+NETLINK_ALIGN(len)))
 #define NLMSG_RTAOK(rta,nlh)	RTA_OK(rta,NLMSG_DATAEND(nlh))


Could you try if this fixes it?

You will probably need to 'make clean' or at least force recompilation
of src/network/{getifaddrs,if_nameindex,netlink}.c as the netlink.h
dependency is not picked up by the makefile automatically.

@dalias, if the above looks good to you, I am happy to submit properly
formatted git patch for it.

Thanks,
Timo