From patchwork Fri Jan 24 12:43:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Vincent Hou X-Patchwork-Id: 1228815 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=uclibc-ng.org (client-ip=2a00:1828:2000:679::23; helo=helium.openadk.org; envelope-from=devel-bounces@uclibc-ng.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com X-Greylist: delayed 341 seconds by postgrey-1.36 at bilbo; Fri, 24 Jan 2020 23:49:35 AEDT Received: from helium.openadk.org (helium.openadk.org [IPv6:2a00:1828:2000:679::23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 483zVl5rJRz9sNF for ; Fri, 24 Jan 2020 23:49:35 +1100 (AEDT) Received: from helium.openadk.org (localhost [IPv6:::1]) by helium.openadk.org (Postfix) with ESMTP id 88D3610065; Fri, 24 Jan 2020 13:43:40 +0100 (CET) X-Original-To: devel@uclibc-ng.org Delivered-To: devel@helium.openadk.org Received: from Sheltie.local (ext-104-36-248-13.arubanetworks.com [104.36.248.13]) by helium.openadk.org (Postfix) with ESMTP id 40CD310065; Fri, 24 Jan 2020 13:43:32 +0100 (CET) Received: by Sheltie.local (Postfix, from userid 502) id 6BD62201745433; Fri, 24 Jan 2020 20:43:30 +0800 (CST) From: Vincent Hou To: devel@uclibc-ng.org Date: Fri, 24 Jan 2020 20:43:24 +0800 Message-Id: <20200124124324.13732-1-vincent.houyi@gmail.com> X-Mailer: git-send-email 2.18.0 MIME-Version: 1.0 Cc: Vincent Hou Subject: [uclibc-ng-devel] [PATCH] Fix map_newlink abort when interface list changes during getifaddrs X-BeenThere: devel@uclibc-ng.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: uClibc-ng Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devel-bounces@uclibc-ng.org Sender: "devel" 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 --- 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