[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#582383: libc6: When creating/removing a lot of interfaces it's possible to trigger an abort in glibc's getifaddrs



Package: libc6
Version: 2.7-18lenny2
Severity: important
Tags: patch

Because of this bug in libc, ladvd (and perhaps other softwares) can
trigger an abort() in libc when network interfaces change (up/down,
create destroy) rapidly.  I've noticed this with ladvd, and contacted
its developer to clarify the issue, thus please allow me to quote from a
conversation with Sten Spans - author and maintainer of the ladvd
software -, as this will describe the problem precisely:

> Sten Spans 
I think I've found the issue. When creating/removing a lot of
interfaces I'm able to trigger an abort in glibc's getifaddrs:

#0  0x00007fd2727604b5 in *__GI_raise (sig=<value optimized out>)
    at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#1  0x00007fd272763f50 in *__GI_abort () at abort.c:92
#2  0x00007fd27282e9d8 in map_newlink (ifap=<value optimized out>)
    at ../sysdeps/unix/sysv/linux/ifaddrs.c:320
#3  *__GI_getifaddrs (ifap=<value optimized out>)
    at ../sysdeps/unix/sysv/linux/ifaddrs.c:751
#4  0x0000000000407987 in netif_fetch (ifc=<value optimized out>,
    ifl=<value optimized out>, sysinfo=0x616c40, netifs=0x616c20)
    at netif.c:140

This issue has already been fixed in upstream glibc a few weeks ago
with the following patch:

http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=b8b14c4cc38883032b8ebae50c9a8b3efd256483

It would be great if you could verify that this is indeed the issue.
The sad thing is that I'm not quite sure how to solve this without
patching glibc.
---------------------------------------------------------------------

I was able reproduce this abort on ~180 machines with a lot of interfaces which
are going up/down at a great pace.

The attached patch is from upstream, and is the same as on the above
mentioned sourceware.org link.


-- System Information:
Debian Release: 5.0.4
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: i386 (i686)

Kernel: Linux 2.6.29.6-smp (SMP w/2 CPU cores)
Locale: LANG=en_US, LC_CTYPE=en_US (charmap=ISO-8859-1) (ignored: LC_ALL set to en_US)
Shell: /bin/sh linked to /bin/bash

Versions of packages libc6 depends on:
ii  libgcc1                      1:4.3.2-1.1 GCC support library

libc6 recommends no packages.

Versions of packages libc6 suggests:
pn  glibc-doc                   <none>       (no description available)
ii  libc6-i686                  2.7-18lenny2 GNU C Library: Shared libraries [i
ii  locales                     2.7-18lenny2 GNU C Library: National Language (

-- debconf information:
  glibc/upgrade: true
  glibc/restart-failed:
  glibc/restart-services:
>From b8b14c4cc38883032b8ebae50c9a8b3efd256483 Mon Sep 17 00:00:00 2001
From: Ulrich Drepper <drepper@redhat.com>
Date: Sat, 3 Apr 2010 20:36:59 -0700
Subject: [PATCH] Fix changes to interface list during getifaddrs calls.

---
 ChangeLog                         |   10 +++++++
 sysdeps/unix/sysv/linux/ifaddrs.c |   55 +++++++++++++++++++++++++++++--------
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 6420b9d..2b735fc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2010-04-03  Ulrich Drepper  <drepper@redhat.com>
+
+	[BZ #11387]
+	* sysdeps/unix/sysv/linux/ifaddrs.c (map_newlin): Don't abort on
+	unknown interface, return -1.
+	(getifaddrs_internal): Rename from getifaddrs.  Handle errors in
+	map_newlink be returning -EAGAIN.
+	(getifaddrs): If -EAGAIN is returned from getifaddrs_internal try
+	again.
+
 2010-03-25  Ryan S. Arnold  <rsa@us.ibm.com>
 
 	* sysdeps/unix/sysv/linux/getsysstats.c (next_line): Remove
diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c
index 149bd1c..84f223d 100644
--- a/sysdeps/unix/sysv/linux/ifaddrs.c
+++ b/sysdeps/unix/sysv/linux/ifaddrs.c
@@ -1,5 +1,5 @@
 /* getifaddrs -- get names and addresses of all network interfaces
-   Copyright (C) 2003-2008, 2009 Free Software Foundation, Inc.
+   Copyright (C) 2003-2008, 2009, 2010 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -315,17 +315,19 @@ map_newlink (int index, struct ifaddrs_storage *ifas, int *map, int max)
       else if (map[i] == index)
 	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;
@@ -481,6 +483,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))
@@ -565,9 +574,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;
@@ -747,9 +758,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
@@ -826,6 +841,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)
 
 
-- 
1.7.0.1


Reply to: