[AX25]: UID fixes

 o Brown paperbag bug - ax25_findbyuid() was always returning a NULL pointer
   as the result.  Breaks ROSE completly and AX.25 if UID policy set to deny.

 o While the list structure of AX.25's UID to callsign mapping table was
   properly protected by a spinlock, it's elements were not refcounted
   resulting in a race between removal and usage of an element.

Signed-off-by: Ralf Baechle DL5RB <ralf@linux-mips.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

3 files changed, 55 insertions, 60 deletions

diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index 7d8ecadba668..a5c94f11547c 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1002,7 +1002,8 @@ static int ax25_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
         struct sock *sk = sock->sk;
         struct full_sockaddr_ax25 *addr = (struct full_sockaddr_ax25 *)uaddr;
         ax25_dev *ax25_dev = NULL;
-        ax25_address *call;
+        ax25_uid_assoc *user;
+        ax25_address call;
         ax25_cb *ax25;
         int err = 0;
 
@@ -1021,9 +1022,15 @@ static int ax25_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
         if (addr->fsa_ax25.sax25_family != AF_AX25)
                 return -EINVAL;
 
-        call = ax25_findbyuid(current->euid);
-        if (call == NULL && ax25_uid_policy && !capable(CAP_NET_ADMIN)) {
-                return -EACCES;
+        user = ax25_findbyuid(current->euid);
+        if (user) {
+                call = user->call;
+                ax25_uid_put(user);
+        } else {
+                if (ax25_uid_policy && !capable(CAP_NET_ADMIN))
+                        return -EACCES;
+
+                call = addr->fsa_ax25.sax25_call;
         }
 
         lock_sock(sk);
@@ -1034,10 +1041,7 @@ static int ax25_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
                 goto out;
         }
 
-        if (call == NULL)
-                ax25->source_addr = addr->fsa_ax25.sax25_call;
-        else
-                ax25->source_addr = *call;
+        ax25->source_addr = call;
 
         /*
          * User already set interface with SO_BINDTODEVICE
diff --git a/net/ax25/ax25_route.c b/net/ax25/ax25_route.c
index 44b99b1ff9f8..c288526da4ce 100644
--- a/net/ax25/ax25_route.c
+++ b/net/ax25/ax25_route.c
@@ -422,8 +422,8 @@ static inline void ax25_adjust_path(ax25_address *addr, ax25_digi *digipeat)
  */
 int ax25_rt_autobind(ax25_cb *ax25, ax25_address *addr)
 {
+        ax25_uid_assoc *user;
         ax25_route *ax25_rt;
-        ax25_address *call;
         int err;
 
         if ((ax25_rt = ax25_get_route(addr, NULL)) == NULL)
@@ -434,16 +434,18 @@ int ax25_rt_autobind(ax25_cb *ax25, ax25_address *addr)
                 goto put;
         }
 
-        if ((call = ax25_findbyuid(current->euid)) == NULL) {
+        user = ax25_findbyuid(current->euid);
+        if (user) {
+                ax25->source_addr = user->call;
+                ax25_uid_put(user);
+        } else {
                 if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) {
                         err = -EPERM;
                         goto put;
                 }
-                call = (ax25_address *)ax25->ax25_dev->dev->dev_addr;
+                ax25->source_addr = *(ax25_address *)ax25->ax25_dev->dev->dev_addr;
         }
 
-        ax25->source_addr = *call;
-
         if (ax25_rt->digipeat != NULL) {
                 if ((ax25->digipeat = kmalloc(sizeof(ax25_digi), GFP_ATOMIC)) == NULL) {
                         err = -ENOMEM;
diff --git a/net/ax25/ax25_uid.c b/net/ax25/ax25_uid.c
index cea6b7d19729..a8b3822f3ee4 100644
--- a/net/ax25/ax25_uid.c
+++ b/net/ax25/ax25_uid.c
@@ -28,6 +28,7 @@
 #include <linux/fcntl.h>
 #include <linux/mm.h>
 #include <linux/interrupt.h>
+#include <linux/list.h>
 #include <linux/notifier.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
@@ -41,38 +42,41 @@
  *      Callsign/UID mapper. This is in kernel space for security on multi-amateur machines.
  */
 
-static ax25_uid_assoc *ax25_uid_list;
+HLIST_HEAD(ax25_uid_list);
 static DEFINE_RWLOCK(ax25_uid_lock);
 
 int ax25_uid_policy = 0;
 
-ax25_address *ax25_findbyuid(uid_t uid)
+ax25_uid_assoc *ax25_findbyuid(uid_t uid)
 {
-        ax25_uid_assoc *ax25_uid;
-        ax25_address *res = NULL;
+        ax25_uid_assoc *ax25_uid, *res = NULL;
+        struct hlist_node *node;
 
         read_lock(&ax25_uid_lock);
-        for (ax25_uid = ax25_uid_list; ax25_uid != NULL; ax25_uid = ax25_uid->next) {
+        ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
                 if (ax25_uid->uid == uid) {
-                        res = &ax25_uid->call;
+                        ax25_uid_hold(ax25_uid);
+                        res = ax25_uid;
                         break;
                 }
         }
         read_unlock(&ax25_uid_lock);
 
-        return NULL;
+        return res;
 }
 
 int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 {
-        ax25_uid_assoc *s, *ax25_uid;
+        ax25_uid_assoc *ax25_uid;
+        struct hlist_node *node;
+        ax25_uid_assoc *user;
         unsigned long res;
 
         switch (cmd) {
         case SIOCAX25GETUID:
                 res = -ENOENT;
                 read_lock(&ax25_uid_lock);
-                for (ax25_uid = ax25_uid_list; ax25_uid != NULL; ax25_uid = ax25_uid->next) {
+                ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
                         if (ax25cmp(&sax->sax25_call, &ax25_uid->call) == 0) {
                                 res = ax25_uid->uid;
                                 break;
@@ -85,19 +89,22 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
         case SIOCAX25ADDUID:
                 if (!capable(CAP_NET_ADMIN))
                         return -EPERM;
-                if (ax25_findbyuid(sax->sax25_uid))
+                user = ax25_findbyuid(sax->sax25_uid);
+                if (user) {
+                        ax25_uid_put(user);
                         return -EEXIST;
+                }
                 if (sax->sax25_uid == 0)
                         return -EINVAL;
                 if ((ax25_uid = kmalloc(sizeof(*ax25_uid), GFP_KERNEL)) == NULL)
                         return -ENOMEM;
 
+                atomic_set(&ax25_uid->refcount, 1);
                 ax25_uid->uid  = sax->sax25_uid;
                 ax25_uid->call = sax->sax25_call;
 
                 write_lock(&ax25_uid_lock);
-                ax25_uid->next = ax25_uid_list;
-                ax25_uid_list  = ax25_uid;
+                hlist_add_head(&ax25_uid->uid_node, &ax25_uid_list);
                 write_unlock(&ax25_uid_lock);
 
                 return 0;
@@ -106,34 +113,21 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
                 if (!capable(CAP_NET_ADMIN))
                         return -EPERM;
 
+                ax25_uid = NULL;
                 write_lock(&ax25_uid_lock);
-                for (ax25_uid = ax25_uid_list; ax25_uid != NULL; ax25_uid = ax25_uid->next) {
-                        if (ax25cmp(&sax->sax25_call, &ax25_uid->call) == 0) {
+                ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
+                        if (ax25cmp(&sax->sax25_call, &ax25_uid->call) == 0)
                                 break;
-                        }
                 }
                 if (ax25_uid == NULL) {
                         write_unlock(&ax25_uid_lock);
                         return -ENOENT;
                 }
-                if ((s = ax25_uid_list) == ax25_uid) {
-                        ax25_uid_list = s->next;
-                        write_unlock(&ax25_uid_lock);
-                        kfree(ax25_uid);
-                        return 0;
-                }
-                while (s != NULL && s->next != NULL) {
-                        if (s->next == ax25_uid) {
-                                s->next = ax25_uid->next;
-                                write_unlock(&ax25_uid_lock);
-                                kfree(ax25_uid);
-                                return 0;
-                        }
-                        s = s->next;
-                }
+                hlist_del_init(&ax25_uid->uid_node);
+                ax25_uid_put(ax25_uid);
                 write_unlock(&ax25_uid_lock);
 
-                return -ENOENT;
+                return 0;
 
         default:
                 return -EINVAL;
@@ -147,13 +141,11 @@ int ax25_uid_ioctl(int cmd, struct sockaddr_ax25 *sax)
 static void *ax25_uid_seq_start(struct seq_file *seq, loff_t *pos)
 {
         struct ax25_uid_assoc *pt;
-        int i = 1;
+        struct hlist_node *node;
+        int i = 0;
 
         read_lock(&ax25_uid_lock);
-        if (*pos == 0)
-                return SEQ_START_TOKEN;
-
-        for (pt = ax25_uid_list; pt != NULL; pt = pt->next) {
+        ax25_uid_for_each(pt, node, &ax25_uid_list) {
                 if (i == *pos)
                         return pt;
                 ++i;
@@ -164,8 +156,9 @@ static void *ax25_uid_seq_start(struct seq_file *seq, loff_t *pos)
 static void *ax25_uid_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
         ++*pos;
-        return (v == SEQ_START_TOKEN) ? ax25_uid_list : 
-                ((struct ax25_uid_assoc *) v)->next;
+
+        return hlist_entry(((ax25_uid_assoc *)v)->uid_node.next,
+                           ax25_uid_assoc, uid_node);
 }
 
 static void ax25_uid_seq_stop(struct seq_file *seq, void *v)
@@ -179,7 +172,6 @@ static int ax25_uid_seq_show(struct seq_file *seq, void *v)
                 seq_printf(seq, "Policy: %d\n", ax25_uid_policy);
         else {
                 struct ax25_uid_assoc *pt = v;
-                
 
                 seq_printf(seq, "%6d %s\n", pt->uid, ax2asc(&pt->call));
         }
@@ -213,16 +205,13 @@ struct file_operations ax25_uid_fops = {
  */
 void __exit ax25_uid_free(void)
 {
-        ax25_uid_assoc *s, *ax25_uid;
+        ax25_uid_assoc *ax25_uid;
+        struct hlist_node *node;
 
         write_lock(&ax25_uid_lock);
-        ax25_uid = ax25_uid_list;
-        while (ax25_uid != NULL) {
-                s        = ax25_uid;
-                ax25_uid = ax25_uid->next;
-
-                kfree(s);
+        ax25_uid_for_each(ax25_uid, node, &ax25_uid_list) {
+                hlist_del_init(&ax25_uid->uid_node);
+                ax25_uid_put(ax25_uid);
         }
-        ax25_uid_list = NULL;
         write_unlock(&ax25_uid_lock);
 }