[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Sat, 2 Jun 2018 10:36:31 -0700
From: Andrei Vagin <avagin@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: musl@...ts.openwall.com
Subject: Re: Endless loop in netlink_msg_to_ifaddr
On Fri, Jun 01, 2018 at 09:44:33PM -0400, Rich Felker wrote:
> On Wed, May 30, 2018 at 03:45:29PM +0300, Timo Teras wrote:
> > 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))
Here is an example of the same macros from the kernel sources:
include/uapi/linux/netlink.h:
#define NLMSG_OK(nlh,len) ((len) >= (int)sizeof(struct nlmsghdr) && \
(nlh)->nlmsg_len >= sizeof(struct nlmsghdr) && \
(nlh)->nlmsg_len <= (len))
It contains one more expression to check that an end of a message is in a buffer.
I think it makes sense to add this expression too.
> >
> > #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 RTA_OK(rta,len) ((len) >= (int)sizeof(struct rtattr) && \
(rta)->rta_len >= sizeof(struct rtattr) && \
(rta)->rta_len <= (len))
> >
> > #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?
>
> I'm still waiting to hear whether this fixed 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.
>
> I don't see anything obvious wrong with the proposed patch, but it
> would be nice to have a better understanding of why it's needed and
> whether this is a workaround for a kernel bug (present in which
> kernels?) or something else.
>
> Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
