Hi Vincent, I have applied your patch, thx.
best regards Waldemar Vincent Hou wrote, > Hi all, > > I've set this patch under review on Patchwork. This is the first time I push > for open source project. Please correct me if this is not the right procedure. > > I couldn't find a stable way to reproduce the issue, but it does happen time > to > time. Not seeing the same issue after applying this patch on my environment. > > Any comments on the patch? > > Thanks! > Vincent > > On Fri, 24 Jan 2020 at 20:43, Vincent Hou <[email protected]> wrote: > > map_newlink() may abort when interface list changed between netlink > request for getting interfaces and getting addresses. This commit is > ported from the same change from glibc commit. > > Signed-off-by: Vincent Hou <[email protected]> > --- > > Hi all, > > App code making a getaddrinfo() call may get a occasional abort from > map_newlink(). The reason of the abort is getifaddrs() uses an out-dated > interface list. > > In getaddrinfo() call, it will call a function getifaddrs() who retrieves > all interfaces and addresses from kernel via netlink, returning an array > of interfaces and addresses. It will then call a function map_newlink() > to build a mapping between the interfaces’ index into the returning array > to the interface id in kernel. Then getifaddrs() will bind the > relationship between interface and the address using the mapping from > previous step. > > The interfaces and addresses are retrieved from two separate netlink > requests. It retrieves interface lists then followed by address list. > Between two requests, kernel may make change to interfaces and addresses. > If a new interface and address is added between the requests, the > newly-added interface doesn’t present in the interface list, but the > new-added address does. So the address will point to an non-existent > interface index. > > This issue is fixed by glibc and this change ports the commit from glibc > (with some minor changes). > > Thanks, > Vincent Hou > libc/inet/ifaddrs.c | 53 +++++++++++++++++++++++++++++++++++---------- > 1 file changed, 42 insertions(+), 11 deletions(-) > > diff --git a/libc/inet/ifaddrs.c b/libc/inet/ifaddrs.c > index 0c9310651..72771d35a 100644 > --- a/libc/inet/ifaddrs.c > +++ b/libc/inet/ifaddrs.c > @@ -339,17 +339,19 @@ map_newlink (int idx, struct ifaddrs_storage *ifas, > int *map, int max) > else if (map[i] == idx) > return i; > } > - /* This should never be reached. If this will be reached, we have > - a very big problem. */ > - abort (); > + > + /* This means interfaces changed inbetween the reading of the > + RTM_GETLINK and RTM_GETADDR information. We have to repeat > + everything. */ > + return -1; > } > > > /* Create a linked list of `struct ifaddrs' structures, one for each > network interface on the host machine. If successful, store the > list in *IFAP and return 0. On errors, return -1 and set `errno'. */ > -int > -getifaddrs (struct ifaddrs **ifap) > +static int > +getifaddrs_internal (struct ifaddrs **ifap) > { > struct netlink_handle nh = { 0, 0, 0, NULL, NULL }; > struct netlink_res *nlp; > @@ -496,6 +498,13 @@ getifaddrs (struct ifaddrs **ifap) > kernel. */ > ifa_index = map_newlink (ifim->ifi_index - 1, ifas, > map_newlink_data, newlink); > + if (__builtin_expect (ifa_index == -1, 0)) > + { > + try_again: > + result = -EAGAIN; > + free (ifas); > + goto exit_free; > + } > ifas[ifa_index].ifa.ifa_flags = ifim->ifi_flags; > > while (RTA_OK (rta, rtasize)) > @@ -580,9 +589,11 @@ getifaddrs (struct ifaddrs **ifap) > that we have holes in the interface part of the list, > but we always have already the interface for this > address. */ > ifa_index = newlink + newaddr_idx; > - ifas[ifa_index].ifa.ifa_flags > - = ifas[map_newlink (ifam->ifa_index - 1, ifas, > - map_newlink_data, > newlink)].ifa.ifa_flags; > + int idx = map_newlink (ifam->ifa_index - 1, ifas, > + map_newlink_data, newlink); > + if (__builtin_expect (idx == -1, 0)) > + goto try_again; > + ifas[ifa_index].ifa.ifa_flags = ifas[idx].ifa.ifa_flags; > if (ifa_index > 0) > ifas[ifa_index - 1].ifa.ifa_next = &ifas[ifa_index].ifa; > ++newaddr_idx; > @@ -768,9 +779,13 @@ getifaddrs (struct ifaddrs **ifap) > /* If we didn't get the interface name with the > address, use the name from the interface entry. */ > if (ifas[ifa_index].ifa.ifa_name == NULL) > - ifas[ifa_index].ifa.ifa_name > - = ifas[map_newlink (ifam->ifa_index - 1, ifas, > - map_newlink_data, > newlink)].ifa.ifa_name; > + { > + int idx = map_newlink (ifam->ifa_index - 1, ifas, > + map_newlink_data, newlink); > + if (__builtin_expect (idx == -1, 0)) > + goto try_again; > + ifas[ifa_index].ifa.ifa_name = ifas[idx].ifa.ifa_name; > + } > > /* Calculate the netmask. */ > if (ifas[ifa_index].ifa.ifa_addr > @@ -850,6 +865,22 @@ getifaddrs (struct ifaddrs **ifap) > > return result; > } > + > + > +/* Create a linked list of `struct ifaddrs' structures, one for each > + network interface on the host machine. If successful, store the > + list in *IFAP and return 0. On errors, return -1 and set `errno'. */ > +int > +getifaddrs (struct ifaddrs **ifap) > +{ > + int res; > + > + do > + res = getifaddrs_internal (ifap); > + while (res == -EAGAIN); > + > + return res; > +} > libc_hidden_def(getifaddrs) > > void > -- > 2.18.0 > > > _______________________________________________ > devel mailing list > [email protected] > https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel _______________________________________________ devel mailing list [email protected] https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
