missing spin_unlock in tcp_v4_get_port

missing spin_unlock in tcp_v4_get_port

[Ted Unangst]

There appears to be a missing spin_unlock in tcp_v4_get_port.

                 do {    rover++;
                         if (rover > high)
                                 rover = low;
                         head = &tcp_bhash[tcp_bhashfn(rover)];
                         spin_lock(&head->lock);
head->lock is acquired.
                         tb_for_each(tb, node, &head->chain)
                                 if (tb->port == rover)
                                         goto next;
we don't find what we want.  break out of while loop.
                         break;
                 next:
                         spin_unlock(&head->lock);
                 } while (--remaining > 0);
                 tcp_port_rover = rover;
                 spin_unlock(&tcp_portalloc_lock);

                 /* Exhausted local port range during search? */
                 ret = 1;
                 if (remaining <= 0)
                         goto fail;
here we go to fail; head->lock is still acquired.
....
fail_unlock:
         spin_unlock(&head->lock);
fail:
         local_bh_enable();
         return ret;

Is this a real bug?  The same code was also copy-pasted into 
tcp_v6_get_port.


-- 
Ted Unangst             www.coverity.com             Coverity, Inc.

Re: missing spin_unlock in tcp_v4_get_port

[David S. Miller]

From: Ted Unangst <tedu@coverity.com>
Subject: missing spin_unlock in tcp_v4_get_port
Date: Mon, 22 Aug 2005 14:17:36 -0700

> There appears to be a missing spin_unlock in tcp_v4_get_port.
> 
>                  do {    rover++;
>                          if (rover > high)
>                                  rover = low;
>                          head = &tcp_bhash[tcp_bhashfn(rover)];
>                          spin_lock(&head->lock);
> head->lock is acquired.
>                          tb_for_each(tb, node, &head->chain)
>                                  if (tb->port == rover)
>                                          goto next;
> we don't find what we want.  break out of while loop.
>                          break;
>                  next:
>                          spin_unlock(&head->lock);
>                  } while (--remaining > 0);
>                  tcp_port_rover = rover;
>                  spin_unlock(&tcp_portalloc_lock);
> 
>                  /* Exhausted local port range during search? */
>                  ret = 1;
>                  if (remaining <= 0)
>                          goto fail;
> here we go to fail; head->lock is still acquired.

Only if remaining <= 0, in which case we broke out of the loop due to
the "while (--remaining > 0)" test, not because of the "break;"
statement, and thus the lock is not held.