[TCP/DCCP]: Randomize port selection
authorStephen Hemminger <shemminger@osdl.org>
Fri, 4 Nov 2005 00:33:23 +0000 (16:33 -0800)
committerArnaldo Carvalho de Melo <acme@mandriva.com>
Sat, 5 Nov 2005 23:23:15 +0000 (21:23 -0200)
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>
include/net/inet_hashtables.h
net/dccp/ipv4.c
net/ipv4/inet_connection_sock.c
net/ipv4/tcp.c
net/ipv4/tcp_ipv4.c
net/ipv6/tcp_ipv6.c

index f50f959683400b78df241e061f64138c0f7a9ff9..07840baa934125280a97a3261fcb25da1840588e 100644 (file)
@@ -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,
index 6298cf58ff9e88b329ddba654f3c4ad79b500bcc..4b9bc81ae1a3a35c6fe91a1c4cea05678c2e0781 100644 (file)
@@ -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);
index 94468a76c5b4e0f70bca33afcf0c5b95cf654a7c..3fe021f1a566ad40353fa8a92e1c6d049409a7ad 100644 (file)
@@ -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
index f3f0013a95804808d1522e89192b0beaefa91a30..72b7c22e1ea5aa23306e69b26857d78224e841b0 100644 (file)
@@ -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;
index c85819d8474bba64dc433ca4fd93684bee7421c5..49d67cd75eddcf07a79946c8204fa1265cbca498 100644 (file)
@@ -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)
index d693cb988b78f99407f529e0a956e099e468c6ef..d746d3b27efb7c84407ff432756e32132f7f49fa 100644 (file)
@@ -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