musl - Re: Endless loop in netlink_msg_to

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180530154529.0bf8f46b@vostro>
Date: Wed, 30 May 2018 15:45:29 +0300
From: Timo Teras <timo.teras@....fi>
To: Matej Kupljen <matej.kupljen@...il.com>
Cc: musl@...ts.openwall.com
Subject: Re: Endless loop in netlink_msg_to_ifaddr

On Wed, 30 May 2018 11:57:03 +0200
Matej Kupljen <matej.kupljen@...il.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

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.