[TCP/DCCP]: Randomize port selection

This patch randomizes the port selected on bind() for connections
to help with possible security attacks. It should also be faster
in most cases because there is no need for a global lock.

Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>

diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index f50f959683400b78df241e061f64138c0f7a9ff9..07840baa934125280a97a3261fcb25da1840588e 100644 (file)
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -125,9 +125,7 @@ struct inet_hashinfo {
        rwlock_t                        lhash_lock ____cacheline_aligned;
        atomic_t                        lhash_users;
        wait_queue_head_t               lhash_wait;
-       spinlock_t                      portalloc_lock;
        kmem_cache_t                    *bind_bucket_cachep;
-       int                             port_rover;
 };
 
 static inline unsigned int inet_ehashfn(const __u32 laddr, const __u16 lport,

diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 6298cf58ff9e88b329ddba654f3c4ad79b500bcc..4b9bc81ae1a3a35c6fe91a1c4cea05678c2e0781 100644 (file)
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -31,8 +31,6 @@ struct inet_hashinfo __cacheline_aligned dccp_hashinfo = {
        .lhash_lock     = RW_LOCK_UNLOCKED,
        .lhash_users    = ATOMIC_INIT(0),
        .lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(dccp_hashinfo.lhash_wait),
-       .portalloc_lock = SPIN_LOCK_UNLOCKED,
-       .port_rover     = 1024 - 1,
 };
 
 EXPORT_SYMBOL_GPL(dccp_hashinfo);
@@ -125,36 +123,15 @@ static int dccp_v4_hash_connect(struct sock *sk)
        int ret;
 
        if (snum == 0) {
-               int rover;
                int low = sysctl_local_port_range[0];
                int high = sysctl_local_port_range[1];
                int remaining = (high - low) + 1;
+               int rover = net_random() % (high - low) + low;
                struct hlist_node *node;
                struct inet_timewait_sock *tw = NULL;
 
                local_bh_disable();
-
-               /* TODO. Actually it is not so bad idea to remove
-                * dccp_hashinfo.portalloc_lock before next submission to
-                * Linus.
-                * As soon as we touch this place at all it is time to think.
-                *
-                * Now it protects single _advisory_ variable
-                * dccp_hashinfo.port_rover, hence it is mostly useless.
-                * Code will work nicely if we just delete it, but
-                * I am afraid in contented case it will work not better or
-                * even worse: another cpu just will hit the same bucket
-                * and spin there.
-                * So some cpu salt could remove both contention and
-                * memory pingpong. Any ideas how to do this in a nice way?
-                */
-               spin_lock(&dccp_hashinfo.portalloc_lock);
-               rover = dccp_hashinfo.port_rover;
-
                do {
-                       rover++;
-                       if ((rover < low) || (rover > high))
-                               rover = low;
                        head = &dccp_hashinfo.bhash[inet_bhashfn(rover,
                                                    dccp_hashinfo.bhash_size)];
                        spin_lock(&head->lock);
@@ -187,9 +164,9 @@ static int dccp_v4_hash_connect(struct sock *sk)
 
                next_port:
                        spin_unlock(&head->lock);
+                       if (++rover > high)
+                               rover = low;
                } while (--remaining > 0);
-               dccp_hashinfo.port_rover = rover;
-               spin_unlock(&dccp_hashinfo.portalloc_lock);
 
                local_bh_enable();
 
@@ -197,9 +174,6 @@ static int dccp_v4_hash_connect(struct sock *sk)
 
 ok:
                /* All locks still held and bhs disabled */
-               dccp_hashinfo.port_rover = rover;
-               spin_unlock(&dccp_hashinfo.portalloc_lock);
-
                inet_bind_hash(sk, tb, rover);
                if (sk_unhashed(sk)) {
                        inet_sk(sk)->sport = htons(rover);

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 94468a76c5b4e0f70bca33afcf0c5b95cf654a7c..3fe021f1a566ad40353fa8a92e1c6d049409a7ad 100644 (file)
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -78,17 +78,9 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
                int low = sysctl_local_port_range[0];
                int high = sysctl_local_port_range[1];
                int remaining = (high - low) + 1;
-               int rover;
+               int rover = net_random() % (high - low) + low;
 
-               spin_lock(&hashinfo->portalloc_lock);
-               if (hashinfo->port_rover < low)
-                       rover = low;
-               else
-                       rover = hashinfo->port_rover;
                do {
-                       rover++;
-                       if (rover > high)
-                               rover = low;
                        head = &hashinfo->bhash[inet_bhashfn(rover, hashinfo->bhash_size)];
                        spin_lock(&head->lock);
                        inet_bind_bucket_for_each(tb, node, &head->chain)
@@ -97,9 +89,9 @@ int inet_csk_get_port(struct inet_hashinfo *hashinfo,
                        break;
                next:
                        spin_unlock(&head->lock);
+                       if (++rover > high)
+                               rover = low;
                } while (--remaining > 0);
-               hashinfo->port_rover = rover;
-               spin_unlock(&hashinfo->portalloc_lock);
 
                /* Exhausted local port range during search?  It is not
                 * possible for us to be holding one of the bind hash

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f3f0013a95804808d1522e89192b0beaefa91a30..72b7c22e1ea5aa23306e69b26857d78224e841b0 100644 (file)
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2112,7 +2112,6 @@ void __init tcp_init(void)
                sysctl_tcp_max_orphans >>= (3 - order);
                sysctl_max_syn_backlog = 128;
        }
-       tcp_hashinfo.port_rover = sysctl_local_port_range[0] - 1;
 
        sysctl_tcp_mem[0] =  768 << order;
        sysctl_tcp_mem[1] = 1024 << order;

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c85819d8474bba64dc433ca4fd93684bee7421c5..49d67cd75eddcf07a79946c8204fa1265cbca498 100644 (file)
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -93,8 +93,6 @@ struct inet_hashinfo __cacheline_aligned tcp_hashinfo = {
        .lhash_lock     = RW_LOCK_UNLOCKED,
        .lhash_users    = ATOMIC_INIT(0),
        .lhash_wait     = __WAIT_QUEUE_HEAD_INITIALIZER(tcp_hashinfo.lhash_wait),
-       .portalloc_lock = SPIN_LOCK_UNLOCKED,
-       .port_rover     = 1024 - 1,
 };
 
 static int tcp_v4_get_port(struct sock *sk, unsigned short snum)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index d693cb988b78f99407f529e0a956e099e468c6ef..d746d3b27efb7c84407ff432756e32132f7f49fa 100644 (file)
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -114,16 +114,9 @@ static int tcp_v6_get_port(struct sock *sk, unsigned short snum)
                int low = sysctl_local_port_range[0];
                int high = sysctl_local_port_range[1];
                int remaining = (high - low) + 1;
-               int rover;
+               int rover = net_random() % (high - low) + low;
 
-               spin_lock(&tcp_hashinfo.portalloc_lock);
-               if (tcp_hashinfo.port_rover < low)
-                       rover = low;
-               else
-                       rover = tcp_hashinfo.port_rover;
-               do {    rover++;
-                       if (rover > high)
-                               rover = low;
+               do {
                        head = &tcp_hashinfo.bhash[inet_bhashfn(rover, tcp_hashinfo.bhash_size)];
                        spin_lock(&head->lock);
                        inet_bind_bucket_for_each(tb, node, &head->chain)
@@ -132,9 +125,9 @@ static int tcp_v6_get_port(struct sock *sk, unsigned short snum)
                        break;
                next:
                        spin_unlock(&head->lock);
+                       if (++rover > high)
+                               rover = low;
                } while (--remaining > 0);
-               tcp_hashinfo.port_rover = rover;
-               spin_unlock(&tcp_hashinfo.portalloc_lock);
 
                /* Exhausted local port range during search?  It is not
                 * possible for us to be holding one of the bind hash