From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 69287 invoked by alias); 29 Jun 2018 06:58:22 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 69269 invoked by uid 89); 29 Jun 2018 06:58:21 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=seq, inject, interrupted, reproducible X-HELO: mail-yb0-f194.google.com MIME-Version: 1.0 References: <20180615132915.83940-1-dalvarez@redhat.com> <2a3d6cc5-b05e-434f-3059-992da1dbd219@linaro.org> In-Reply-To: <2a3d6cc5-b05e-434f-3059-992da1dbd219@linaro.org> From: Daniel Alvarez Sanchez Date: Fri, 29 Jun 2018 06:58:00 -0000 Message-ID: Subject: Re: [PATCH] [BZ #21812] getifaddrs() Don't return ifa entries with NULL names To: adhemerval.zanella@linaro.org Cc: libc-alpha@sourceware.org Content-Type: text/plain; charset="UTF-8" X-SW-Source: 2018-06/txt/msg00934.txt.bz2 Thanks a lot for reviewing. It'd be nice to get this in soon as it can happen easily on systems with lots of interfaces. Here it's a reproducer: $ sudo unshare -n # cat create-links.sh #!/bin/bash for i in {1..512}; do ifname="dum$i"; ip li add $ifname type dummy; ip li set $ifname up; done # ./create-links.sh # cat loop-add-del-link.sh #!/bin/bash while :; do ifname="test$RANDOM"; ip li add $ifname type dummy; ip li set $ifname up; sleep 0.1; ip li del $ifname; done # ./loop-add-del-link.sh & [1] 6260 # ./loop-add-del-link.sh & [2] 6314 # ./loop-add-del-link.sh & [3] 6374 # ./loop-add-del-link.sh & [4] 6476 # ./loop-add-del-link.sh & [5] 6628 # cat dalvarez-tester.c #include #include #include #include #include int main(void) { struct ifaddrs *ifaddr, *ifa; for (;;) { if (getifaddrs(&ifaddr) == -1) { perror("getifaddrs"); return EXIT_FAILURE; } /* Walk through linked list, maintaining head pointer so we can free list later */ for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) { if (ifa->ifa_name == NULL) { printf("Gotcha!!!!\n"); return EXIT_FAILURE; } } freeifaddrs(ifaddr); usleep(10); } return EXIT_SUCCESS; } On Fri, Jun 15, 2018 at 9:13 PM Adhemerval Zanella wrote: > > > > On 15/06/2018 10:29, Daniel Alvarez wrote: > > Due to bug 21812, a lookup operation in map_newlink() turns out > > into an insert because of holes in the interface part of the map. > > This leads to incorrectly set the name of the interface to NULL when > > the interface is not present for the address being processed (most > > likely because the interface was added between the RTM_GETLINK and > > RTM_GETADDR calls to the kernel). When such changes are detected > > by the kernel, it'll mark the dump as "inconsistent" by setting > > NLM_F_DUMP_INTR flag on the next netlink message. > > > > This patch checks this condition and retries the whole operation. > > Hopes are that next time the interface corresponding to the address > > entry is present in the list and correct name is returned. > > > > Signed-off-by: Daniel Alvarez > > Signed-off-by: Jakub Sitnicki > > We don't use DCO, instead we use copyright assignments. However the > change below seems to fit as a trivial change. > > Patch looks good and I can't think in a easy way to add testcase which > is guarantee to be reproducible (maybe by using a network namespace and > force inject an interface so kernel will set NLM_F_DUMP_INTR?). > > > --- > > sysdeps/unix/sysv/linux/ifaddrs.c | 8 ++++++++ > > 1 file changed, 8 insertions(+) > > > > diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c > > index 32381f54e4..5659c9605b 100644 > > --- a/sysdeps/unix/sysv/linux/ifaddrs.c > > +++ b/sysdeps/unix/sysv/linux/ifaddrs.c > > @@ -370,6 +370,14 @@ getifaddrs_internal (struct ifaddrs **ifap) > > if ((pid_t) nlh->nlmsg_pid != nh.pid || nlh->nlmsg_seq != nlp->seq) > > continue; > > > > + /* If the dump got interrupted, we can't relay on the results so > > + try again. */ > > + if (nlh->nlmsg_flags & NLM_F_DUMP_INTR) > > + { > > + result = -EAGAIN; > > + goto exit_free; > > + } > > + > > if (nlh->nlmsg_type == NLMSG_DONE) > > break; /* ok */ > > > >