From patchwork Tue Nov 3 13:40:15 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 539402 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 5A833140291 for ; Wed, 4 Nov 2015 00:40:30 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b=PoAS8LP8; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; q=dns; s=default; b=RbTY +tQFrdzfyKz3wtF7FZZ64g0t8wOMEhVq+V527wZ/tKVGuWjy3NlDjdrFAliwT4Ly 3AQCwGkgBeBFBf25JvwfVgOVnHSC4GQrgQll0T6HmtgTvxY7sK3mVPZLy2cmUd7L W52GfrQRKkR/uaB/C7t2+Av6pMSq/5Hb1sZlim8= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:subject:to:references:cc:from:message-id:date :mime-version:in-reply-to:content-type; s=default; bh=m1SLSmmN0A U1qGDsH874yx9tGxQ=; b=PoAS8LP85kSNeXR7RPv/VAjk845J2hMmR9yCDI7l6d MPxAhMcfXiVQFroE49Zq4bxP9NU2Fcdyg1Ac4tPI/Q7Pc2RBaeWxrc6JnEJDHJRR MZ7R30zubPA6K6krx+b/rzictR5HKlHE+y8Ow0Iu6znK1mS73KAnBvG0b+pqQgCa Q= Received: (qmail 93705 invoked by alias); 3 Nov 2015 13:40:23 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 93684 invoked by uid 89); 3 Nov 2015 13:40:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Subject: [PATCH v2] glibc: Terminate process on invalid netlink response from kernel [BZ #12926] To: GNU C Library References: <562A9391.1040806@redhat.com> <20151024042203.GQ26317@vapier.lan> Cc: Hannes Sowa , netdev@vger.kernel.org From: Florian Weimer X-Enigmail-Draft-Status: N1010 Message-ID: <5638B93F.3090202@redhat.com> Date: Tue, 3 Nov 2015 14:40:15 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151024042203.GQ26317@vapier.lan> On 10/24/2015 06:22 AM, Mike Frysinger wrote: > On 23 Oct 2015 22:07, Florian Weimer wrote: >> --- /dev/null >> +++ b/sysdeps/unix/sysv/linux/netlink_assert_response.c >> @@ -0,0 +1,100 @@ >> +/* Copyright (C) 2015 Free Software Foundation, Inc. >> + This file is part of the GNU C Library. > > guess we like to have the first line be a short desc of the file Added. >> +static int >> +get_address_family (int fd) >> +{ >> ... >> + return sa.ss_family; > ss_family is of type sa_family_t, not int ... not a big deal, but the > two do differ in sign ... Thanks. I added static asserts to make sure that the actual type does not cause problems with the use of -1 and an int return value. I do not want to use SO_DOMAIN here because I expect this to eventually move into generic code because we probably should do similar checking on other internally-used sockets (where reporting impossible errors to the caller would be grossly misleading). I'm still waiting for comments from the kernel people. :) Florian Terminate process on invalid netlink response from kernel [BZ The recvmsg system calls for netlink sockets have been particularly prone to picking up unrelated data after a file descriptor race (where the descriptor is closed and reopened concurrently in a multi-threaded process, as the result of a file descriptor management issue elsewhere). This commit adds additional error checking and aborts the process if a datagram of unexpected length (without the netlink header) is received, or an error code which cannot happen due to the way the netlink socket is used. 2015-11-03 Florian Weimer [BZ #12926] Terminate process on invalid netlink response. * sysdeps/unix/sysv/linux/netlinkaccess.h (__netlink_assert_response): Declare. * sysdeps/unix/sysv/linux/netlink_assert_response.c: New file. * sysdeps/unix/sysv/linux/Makefile [$(subdir) == inet] (sysdep_routines): Add netlink_assert_response. * sysdeps/unix/sysv/linux/check_native.c (__check_native): Call __netlink_assert_response. * sysdeps/unix/sysv/linux/check_pf.c (make_request): Likewise. * sysdeps/unix/sysv/linux/ifaddrs.c (__netlink_request): Likewise. * sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add __netlink_assert_response. diff --git a/ChangeLog b/ChangeLog index ab7aa69..0b8ed92 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,19 @@ +2015-11-03 Florian Weimer + + [BZ #12926] + Terminate process on invalid netlink response. + * sysdeps/unix/sysv/linux/netlinkaccess.h + (__netlink_assert_response): Declare. + * sysdeps/unix/sysv/linux/netlink_assert_response.c: New file. + * sysdeps/unix/sysv/linux/Makefile [$(subdir) == inet] + (sysdep_routines): Add netlink_assert_response. + * sysdeps/unix/sysv/linux/check_native.c (__check_native): Call + __netlink_assert_response. + * sysdeps/unix/sysv/linux/check_pf.c (make_request): Likewise. + * sysdeps/unix/sysv/linux/ifaddrs.c (__netlink_request): Likewise. + * sysdeps/unix/sysv/linux/Versions (GLIBC_PRIVATE): Add + __netlink_assert_response. + 2015-11-02 Joseph Myers * math/libm-test.inc (modf_test_data): Add more tests. diff --git a/NEWS b/NEWS index 896eb02..d3738df 100644 --- a/NEWS +++ b/NEWS @@ -24,6 +24,14 @@ Version 2.23 19095, 19124, 19125, 19129, 19134, 19137, 19156, 19174, 19181, 19189, 19201. +* getaddrinfo now detects certain invalid responses on an internal netlink + socket. If such responses are received, an affected process will + terminate with an error message of "Unexpected error on netlink + descriptor " or "Unexpected netlink response of size on + descriptor ". The most likely cause for these errors is a + multi-threaded application which erroneously closes and reuses the netlink + file descriptor while it is used by getaddrinfo. + * A defect in the malloc implementation, present since glibc 2.15 (2012) or glibc 2.10 via --enable-experimental-malloc (2009), could result in the unnecessary serialization of memory allocation requests across threads. diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile index 2c67a66..d6cc529 100644 --- a/sysdeps/unix/sysv/linux/Makefile +++ b/sysdeps/unix/sysv/linux/Makefile @@ -151,6 +151,7 @@ sysdep_headers += netinet/if_fddi.h netinet/if_tr.h \ netipx/ipx.h netash/ash.h netax25/ax25.h netatalk/at.h \ netrom/netrom.h netpacket/packet.h netrose/rose.h \ neteconet/ec.h netiucv/iucv.h +sysdep_routines += netlink_assert_response endif # Don't compile the ctype glue code, since there is no old non-GNU C library. diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions index 16bb281..202ffcc 100644 --- a/sysdeps/unix/sysv/linux/Versions +++ b/sysdeps/unix/sysv/linux/Versions @@ -169,5 +169,7 @@ libc { GLIBC_PRIVATE { # functions used in other libraries __syscall_rt_sigqueueinfo; + # functions used by nscd + __netlink_assert_response; } } diff --git a/sysdeps/unix/sysv/linux/check_native.c b/sysdeps/unix/sysv/linux/check_native.c index eaefca1..d04c8f2 100644 --- a/sysdeps/unix/sysv/linux/check_native.c +++ b/sysdeps/unix/sysv/linux/check_native.c @@ -35,6 +35,7 @@ #include +#include "netlinkaccess.h" void __check_native (uint32_t a1_index, int *a1_native, @@ -117,6 +118,7 @@ __check_native (uint32_t a1_index, int *a1_native, }; ssize_t read_len = TEMP_FAILURE_RETRY (__recvmsg (fd, &msg, 0)); + __netlink_assert_response (fd, read_len); if (read_len < 0) goto out_fail; diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c index f072fb3..af4fdf8 100644 --- a/sysdeps/unix/sysv/linux/check_pf.c +++ b/sysdeps/unix/sysv/linux/check_pf.c @@ -36,6 +36,7 @@ #include #include +#include "netlinkaccess.h" #ifndef IFA_F_HOMEADDRESS # define IFA_F_HOMEADDRESS 0 @@ -164,7 +165,8 @@ make_request (int fd, pid_t pid) }; ssize_t read_len = TEMP_FAILURE_RETRY (__recvmsg (fd, &msg, 0)); - if (read_len <= 0) + __netlink_assert_response (fd, read_len); + if (read_len < 0) goto out_fail; if (msg.msg_flags & MSG_TRUNC) diff --git a/sysdeps/unix/sysv/linux/ifaddrs.c b/sysdeps/unix/sysv/linux/ifaddrs.c index 64b4a1c..768a7ed 100644 --- a/sysdeps/unix/sysv/linux/ifaddrs.c +++ b/sysdeps/unix/sysv/linux/ifaddrs.c @@ -168,6 +168,7 @@ __netlink_request (struct netlink_handle *h, int type) }; read_len = TEMP_FAILURE_RETRY (__recvmsg (h->fd, &msg, 0)); + __netlink_assert_response (h->fd, read_len); if (read_len < 0) goto out_fail; diff --git a/sysdeps/unix/sysv/linux/netlink_assert_response.c b/sysdeps/unix/sysv/linux/netlink_assert_response.c new file mode 100644 index 0000000..b570e93 --- /dev/null +++ b/sysdeps/unix/sysv/linux/netlink_assert_response.c @@ -0,0 +1,106 @@ +/* Check recvmsg results for netlink sockets. + Copyright (C) 2015 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 + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include + +#include "netlinkaccess.h" + +static int +get_address_family (int fd) +{ + struct sockaddr_storage sa; + socklen_t sa_len = sizeof (sa); + if (__getsockname (fd, (struct sockaddr *) &sa, &sa_len) < 0) + return -1; + /* Check that the socket family number is preserved despite in-band + signaling. */ + _Static_assert (sizeof (sa.ss_family) < sizeof (int), "address family size"); + _Static_assert (0 < (__typeof__ (sa.ss_family)) -1, + "address family unsigned"); + return sa.ss_family; +} + +void +internal_function +__netlink_assert_response (int fd, ssize_t result) +{ + if (result < 0) + { + /* Check if the error is unexpected. */ + bool terminate = false; + int error_code = errno; + int family = get_address_family (fd); + if (family != AF_NETLINK) + /* If the address family does not match (or getsockname + failed), report the original error. */ + terminate = true; + else if (error_code == EBADF + || error_code == ENOTCONN + || error_code == ENOTSOCK + || error_code == ECONNREFUSED) + /* These errors indicate that the descriptor is not a + connected socket. */ + terminate = true; + else if (error_code == EAGAIN || error_code == EWOULDBLOCK) + { + /* The kernel might return EAGAIN for other reasons than a + non-blocking socket. But if the socket is not blocking, + it is not ours, so report the error. */ + int mode = __fcntl (fd, F_GETFL, 0); + if (mode < 0 || (mode & O_NONBLOCK) != 0) + terminate = true; + } + if (terminate) + { + char message[200]; + if (family < 0) + __snprintf (message, sizeof (message), + "Unexpected error %d on netlink descriptor %d", + error_code, fd); + else + __snprintf (message, sizeof (message), + "Unexpected error %d on netlink descriptor %d" + " (address family %d)", + error_code, fd, family); + __libc_fatal (message); + } + else + /* Restore orignal errno value. */ + __set_errno (error_code); + } + else if (result < sizeof (struct nlmsghdr)) + { + char message[200]; + int family = get_address_family (fd); + if (family < 0) + __snprintf (message, sizeof (message), + "Unexpected netlink response of size %zd" + " on descriptor %d", + result, fd); + else + __snprintf (message, sizeof (message), + "Unexpected netlink response of size %zd" + " on descriptor %d (address family %d)", + result, fd, family); + __libc_fatal (message); + } +} +libc_hidden_def (__netlink_assert_response) diff --git a/sysdeps/unix/sysv/linux/netlinkaccess.h b/sysdeps/unix/sysv/linux/netlinkaccess.h index c204b67..01ac35c 100644 --- a/sysdeps/unix/sysv/linux/netlinkaccess.h +++ b/sysdeps/unix/sysv/linux/netlinkaccess.h @@ -19,6 +19,7 @@ #define _NETLINKACCESS_H 1 #include +#include #include #include #include @@ -48,5 +49,10 @@ extern void __netlink_close (struct netlink_handle *h); extern void __netlink_free_handle (struct netlink_handle *h); extern int __netlink_request (struct netlink_handle *h, int type); +/* Terminate the process if RESULT is an invalid recvmsg result for + the netlink socket FD. */ +void __netlink_assert_response (int fd, ssize_t result) + internal_function; +libc_hidden_proto (__netlink_assert_response) #endif /* netlinkaccess.h */ -- 2.4.3