From patchwork Thu Oct 21 13:06:23 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Timo Teras X-Patchwork-Id: 68604 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 15A04B6EF1 for ; Fri, 22 Oct 2010 00:07:00 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757929Ab0JUNGm (ORCPT ); Thu, 21 Oct 2010 09:06:42 -0400 Received: from mail-ew0-f46.google.com ([209.85.215.46]:54510 "EHLO mail-ew0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757924Ab0JUNGl (ORCPT ); Thu, 21 Oct 2010 09:06:41 -0400 Received: by ewy7 with SMTP id 7so36855ewy.19 for ; Thu, 21 Oct 2010 06:06:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:from:to:cc:subject :date:message-id:x-mailer:in-reply-to:references:mime-version :content-type:content-transfer-encoding; bh=KyxG974lArY2FFXPhRz3/Yz3grraWeuRs62l24ApPX4=; b=NZL5zDx5lZ+kNUGfwjRRZQ6v7k0lIXrNSR1AjoRR70lsrSm5fojY+59HDC3o8YWctv 6K2p5haj0xNHcaVbVKROdLCL9K8en8zbqNfp9q2QT5CWV1xbaRKD6cmk2wjVPxpNf9yx w7moIvOl5Jusg8p04WamA9B9jf1wzoLrVOfiI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:from:to:cc:subject:date:message-id:x-mailer:in-reply-to :references:mime-version:content-type:content-transfer-encoding; b=s+5XEwHaXcw2sL9R9f/XinJSBqIRdd+RidVK3QsxcpnV3DOCUd8mDYeRT3auADJCuE FuJNENHuF9wUQ3WvIFX/5mxrCBc8dReu8zCRdp0cOH+5M5W9jcB9Q+kg6Q+rg/LuTK8U 31oImrCsMpjRgSuMPRZy55pAuAjPaZz6kgbPE= Received: by 10.14.119.72 with SMTP id m48mr573746eeh.44.1287666400598; Thu, 21 Oct 2010 06:06:40 -0700 (PDT) Received: from vostro.ism.fin.wtbts.net (mail.fi.jw.org [83.145.235.193]) by mx.google.com with ESMTPS id q58sm1759942eeh.21.2010.10.21.06.06.39 (version=SSLv3 cipher=RC4-MD5); Thu, 21 Oct 2010 06:06:40 -0700 (PDT) From: =?UTF-8?q?Timo=20Ter=C3=A4s?= To: David Miller , eric.dumazet@gmail.com, netdev@vger.kernel.org Cc: =?UTF-8?q?Timo=20Ter=C3=A4s?= Subject: [PATCH v2] ipv4: synchronize bind() with RTM_NEWADDR notifications Date: Thu, 21 Oct 2010 16:06:23 +0300 Message-Id: <1287666383-17615-1-git-send-email-timo.teras@iki.fi> X-Mailer: git-send-email 1.7.1 In-Reply-To: <4CC02ABF.8090008@iki.fi> References: <4CC02ABF.8090008@iki.fi> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Otherwise we have race condition to user land: 1. process A: changes IP address 2. process A: kernel sends RTM_NEWADDR (and schedules out) 3. process B: gets notification 4. process B: tries to bind() to new IP, but fails with EADDRNOTAVAIL because FIB is not yet updated and inet_addr_type() in inet_bind() does not recognize the IP as local 5. process A: calls inetaddr_chain notifiers which updates FIB Fix the error path to synchronize with configuration changes and retry the address type check. IPv6 side seems to handle the notifications properly: bind() immediately after RTM_NEWADDR succeeds as expected. This is because ipv6_chk_addr() uses inet6_addr_lst which is updated before address notification. Signed-off-by: Timo Teräs --- Since there was no reply to my question if this is ok, I interpreted it as "maybe". So here's the code for review. Hopefully this helps determining if this is an acceptable fix. net/ipv4/af_inet.c | 18 ++++++++++++++++-- net/ipv6/af_inet6.c | 13 ++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 6a1100c..013ab11 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -481,8 +481,22 @@ int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) addr->sin_addr.s_addr != htonl(INADDR_ANY) && chk_addr_ret != RTN_LOCAL && chk_addr_ret != RTN_MULTICAST && - chk_addr_ret != RTN_BROADCAST) - goto out; + chk_addr_ret != RTN_BROADCAST) { + /* inet_addr_type() does a FIB lookup to check the + * address type. Since FIB is updated after sending + * RTM_NEWADDR notification, an application may end up + * doing bind() before the FIB is updated. To avoid + * returning a false negative, wait for possible ongoing + * address changes to finish by acquiring rtnl lock and + * retry the address type lookup. */ + rtnl_lock(); + rtnl_unlock(); + chk_addr_ret = inet_addr_type(sock_net(sk), addr->sin_addr.s_addr); + if (chk_addr_ret != RTN_LOCAL && + chk_addr_ret != RTN_MULTICAST && + chk_addr_ret != RTN_BROADCAST) + goto out; + } snum = ntohs(addr->sin_port); err = -EACCES; diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c index 56b9bf2..b1a83e1 100644 --- a/net/ipv6/af_inet6.c +++ b/net/ipv6/af_inet6.c @@ -300,7 +300,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) goto out; } - /* Reproduce AF_INET checks to make the bindings consitant */ + /* Reproduce AF_INET checks to make the bindings consistent */ v4addr = addr->sin6_addr.s6_addr32[3]; chk_addr_ret = inet_addr_type(net, v4addr); if (!sysctl_ip_nonlocal_bind && @@ -309,8 +309,15 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len) chk_addr_ret != RTN_LOCAL && chk_addr_ret != RTN_MULTICAST && chk_addr_ret != RTN_BROADCAST) { - err = -EADDRNOTAVAIL; - goto out; + rtnl_lock(); + rtnl_unlock(); + chk_addr_ret = inet_addr_type(net, v4addr); + if (chk_addr_ret != RTN_LOCAL && + chk_addr_ret != RTN_MULTICAST && + chk_addr_ret != RTN_BROADCAST) { + err = -EADDRNOTAVAIL; + goto out; + } } } else { if (addr_type != IPV6_ADDR_ANY) {