From patchwork Tue Jan 11 11:14:22 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 78327 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 9E918B6EE8 for ; Tue, 11 Jan 2011 22:14:33 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755770Ab1AKLO3 (ORCPT ); Tue, 11 Jan 2011 06:14:29 -0500 Received: from mail-wy0-f174.google.com ([74.125.82.174]:46944 "EHLO mail-wy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754102Ab1AKLO1 (ORCPT ); Tue, 11 Jan 2011 06:14:27 -0500 Received: by wyb28 with SMTP id 28so20340719wyb.19 for ; Tue, 11 Jan 2011 03:14:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=bHQ7CvQ4nipZCtonpOsATnJfVC4ND23wrdwbFVJUUcI=; b=kgDGrdUWqo+MW64WHLImkKZgnTPDa8FoifCe7fcsBVe4bUeuK3lumBNwboQNvbUAbz UGsgCRh1JfI6xAlor/Oxcl6/TYUOLBqQd6/F1uR6xVLF2ITNVSyZO+iPD/d///7zK4SF OucFA2bcyCEj6DdxpbuI+i+ANI7yCYLf353Fk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=HYq1OsxCjjD0NYXr0LnnxUrZ0ml6CpIAPZrdzkrILDG2U1o96FZOqcdnEPHWfUGFrz 6VIyP+I7CJHaFyfBQsCXNqwladHaRv85YgfxJ4V8SLD6hJRuhEmyxSprUWwbtApkNXSX 59mkbWvMkpKgfS7HVQtSiIS3ljug6sWEkwFG0= Received: by 10.216.178.132 with SMTP id f4mr2696312wem.62.1294744466509; Tue, 11 Jan 2011 03:14:26 -0800 (PST) Received: from [10.150.51.215] (gw0.net.jmsp.net [212.23.165.14]) by mx.google.com with ESMTPS id f52sm14652672wes.35.2011.01.11.03.14.25 (version=SSLv3 cipher=RC4-MD5); Tue, 11 Jan 2011 03:14:25 -0800 (PST) Subject: [PATCH] tcp: disallow bind() to reuse addr/port From: Eric Dumazet To: Daniel Baluta , David Miller Cc: Gaspar Chilingarov , netdev In-Reply-To: References: <1294140172.3579.81.camel@edumazet-laptop> Date: Tue, 11 Jan 2011 12:14:22 +0100 Message-ID: <1294744462.2927.53.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le mercredi 05 janvier 2011 à 11:00 +0200, Daniel Baluta a écrit : > Going back to my first email, are there any follow ups on your > "tcp: bind() fix when many ports are bound" patch. I've searched > netdev archives but no luck. I might have missed something. > > I really appreciate your help. > I believe following patch should solve the problem. Thanks for reminding us this issue ! I checked FreeBSD : It doesnt allow two sockets (in CLOSE state) bound on same addr/port, even if both have REUSEADDR set [PATCH] tcp: disallow bind() to reuse addr/port inet_csk_bind_conflict() logic currently disallows a bind() if it finds a friend socket (a socket bound on same address/port) satisfying a set of conditions : 1) Current (to be bound) socket doesnt have sk_reuse set OR 2) other socket doesnt have sk_reuse set OR 3) other socket is in LISTEN state We should add the CLOSE state in the 3) condition, in order to avoid two REUSEADDR sockets in CLOSE state with same local address/port, since this can deny further operations. Note : a prior patch tried to address the problem in a different (and buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports are bound). Reported-by: Gaspar Chilingarov Reported-by: Daniel Baluta Signed-off-by: Eric Dumazet --- net/ipv4/inet_connection_sock.c | 5 +++-- net/ipv6/inet6_connection_sock.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index 25e3181..9f6d585 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -73,7 +73,7 @@ int inet_csk_bind_conflict(const struct sock *sk, !sk2->sk_bound_dev_if || sk->sk_bound_dev_if == sk2->sk_bound_dev_if)) { if (!reuse || !sk2->sk_reuse || - sk2->sk_state == TCP_LISTEN) { + ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) { const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2); if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) || sk2_rcv_saddr == sk_rcv_saddr(sk)) @@ -122,7 +122,8 @@ again: (tb->num_owners < smallest_size || smallest_size == -1)) { smallest_size = tb->num_owners; smallest_rover = rover; - if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) { + if (atomic_read(&hashinfo->bsockets) > (high - low) + 1 && + !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) { spin_unlock(&head->lock); snum = smallest_rover; goto have_snum; diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c index e46305d..d144e62 100644 --- a/net/ipv6/inet6_connection_sock.c +++ b/net/ipv6/inet6_connection_sock.c @@ -44,7 +44,7 @@ int inet6_csk_bind_conflict(const struct sock *sk, !sk2->sk_bound_dev_if || sk->sk_bound_dev_if == sk2->sk_bound_dev_if) && (!sk->sk_reuse || !sk2->sk_reuse || - sk2->sk_state == TCP_LISTEN) && + ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) && ipv6_rcv_saddr_equal(sk, sk2)) break; }