[SCTP]: Fix sctp_getsockopt_local_addrs_old() to use local storage.
authorVlad Yasevich <vladislav.yasevich@hp.com>
Sun, 29 Apr 2007 04:09:04 +0000 (21:09 -0700)
committerDavid S. Miller <davem@davemloft.net>
Sun, 29 Apr 2007 04:09:04 +0000 (21:09 -0700)
sctp_getsockopt_local_addrs_old() in net/sctp/socket.c calls
copy_to_user() while the spinlock addr_lock is held. this should not
be done as copy_to_user() might sleep. the call to
sctp_copy_laddrs_to_user() while holding the lock is also problematic
as it calls copy_to_user()

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/sctp/socket.c

index 11938fb..2fc0a92 100644 (file)
@@ -3987,7 +3987,7 @@ static int sctp_getsockopt_peer_addrs(struct sock *sk, int len,
                memcpy(&temp, &from->ipaddr, sizeof(temp));
                sctp_get_pf_specific(sk->sk_family)->addr_v4map(sp, &temp);
                addrlen = sctp_get_af_specific(sk->sk_family)->sockaddr_len;
-               if(space_left < addrlen)
+               if (space_left < addrlen)
                        return -ENOMEM;
                if (copy_to_user(to, &temp, addrlen))
                        return -EFAULT;
@@ -4076,8 +4076,9 @@ done:
 /* Helper function that copies local addresses to user and returns the number
  * of addresses copied.
  */
-static int sctp_copy_laddrs_to_user_old(struct sock *sk, __u16 port, int max_addrs,
-                                       void __user *to)
+static int sctp_copy_laddrs_old(struct sock *sk, __u16 port,
+                                       int max_addrs, void *to,
+                                       int *bytes_copied)
 {
        struct list_head *pos, *next;
        struct sctp_sockaddr_entry *addr;
@@ -4094,10 +4095,10 @@ static int sctp_copy_laddrs_to_user_old(struct sock *sk, __u16 port, int max_add
                sctp_get_pf_specific(sk->sk_family)->addr_v4map(sctp_sk(sk),
                                                                &temp);
                addrlen = sctp_get_af_specific(temp.sa.sa_family)->sockaddr_len;
-               if (copy_to_user(to, &temp, addrlen))
-                       return -EFAULT;
+               memcpy(to, &temp, addrlen);
 
                to += addrlen;
+               *bytes_copied += addrlen;
                cnt ++;
                if (cnt >= max_addrs) break;
        }
@@ -4105,8 +4106,8 @@ static int sctp_copy_laddrs_to_user_old(struct sock *sk, __u16 port, int max_add
        return cnt;
 }
 
-static int sctp_copy_laddrs_to_user(struct sock *sk, __u16 port,
-                                   void __user **to, size_t space_left)
+static int sctp_copy_laddrs(struct sock *sk, __u16 port, void *to,
+                           size_t space_left, int *bytes_copied)
 {
        struct list_head *pos, *next;
        struct sctp_sockaddr_entry *addr;
@@ -4123,14 +4124,14 @@ static int sctp_copy_laddrs_to_user(struct sock *sk, __u16 port,
                sctp_get_pf_specific(sk->sk_family)->addr_v4map(sctp_sk(sk),
                                                                &temp);
                addrlen = sctp_get_af_specific(temp.sa.sa_family)->sockaddr_len;
-               if(space_left<addrlen)
+               if (space_left < addrlen)
                        return -ENOMEM;
-               if (copy_to_user(*to, &temp, addrlen))
-                       return -EFAULT;
+               memcpy(to, &temp, addrlen);
 
-               *to += addrlen;
+               to += addrlen;
                cnt ++;
                space_left -= addrlen;
+               bytes_copied += addrlen;
        }
 
        return cnt;
@@ -4154,6 +4155,8 @@ static int sctp_getsockopt_local_addrs_old(struct sock *sk, int len,
        int addrlen;
        rwlock_t *addr_lock;
        int err = 0;
+       void *addrs;
+       int bytes_copied = 0;
 
        if (len != sizeof(struct sctp_getaddrs_old))
                return -EINVAL;
@@ -4181,6 +4184,15 @@ static int sctp_getsockopt_local_addrs_old(struct sock *sk, int len,
 
        to = getaddrs.addrs;
 
+       /* Allocate space for a local instance of packed array to hold all
+        * the data.  We store addresses here first and then put write them
+        * to the user in one shot.
+        */
+       addrs = kmalloc(sizeof(union sctp_addr) * getaddrs.addr_num,
+                       GFP_KERNEL);
+       if (!addrs)
+               return -ENOMEM;
+
        sctp_read_lock(addr_lock);
 
        /* If the endpoint is bound to 0.0.0.0 or ::0, get the valid
@@ -4190,13 +4202,9 @@ static int sctp_getsockopt_local_addrs_old(struct sock *sk, int len,
                addr = list_entry(bp->address_list.next,
                                  struct sctp_sockaddr_entry, list);
                if (sctp_is_any(&addr->a)) {
-                       cnt = sctp_copy_laddrs_to_user_old(sk, bp->port,
-                                                          getaddrs.addr_num,
-                                                          to);
-                       if (cnt < 0) {
-                               err = cnt;
-                               goto unlock;
-                       }
+                       cnt = sctp_copy_laddrs_old(sk, bp->port,
+                                                  getaddrs.addr_num,
+                                                  addrs, &bytes_copied);
                        goto copy_getaddrs;
                }
        }
@@ -4206,22 +4214,29 @@ static int sctp_getsockopt_local_addrs_old(struct sock *sk, int len,
                memcpy(&temp, &addr->a, sizeof(temp));
                sctp_get_pf_specific(sk->sk_family)->addr_v4map(sp, &temp);
                addrlen = sctp_get_af_specific(temp.sa.sa_family)->sockaddr_len;
-               if (copy_to_user(to, &temp, addrlen)) {
-                       err = -EFAULT;
-                       goto unlock;
-               }
+               memcpy(addrs, &temp, addrlen);
                to += addrlen;
+               bytes_copied += addrlen;
                cnt ++;
                if (cnt >= getaddrs.addr_num) break;
        }
 
 copy_getaddrs:
+       sctp_read_unlock(addr_lock);
+
+       /* copy the entire address list into the user provided space */
+       if (copy_to_user(to, addrs, bytes_copied)) {
+               err = -EFAULT;
+               goto error;
+       }
+
+       /* copy the leading structure back to user */
        getaddrs.addr_num = cnt;
        if (copy_to_user(optval, &getaddrs, sizeof(struct sctp_getaddrs_old)))
                err = -EFAULT;
 
-unlock:
-       sctp_read_unlock(addr_lock);
+error:
+       kfree(addrs);
        return err;
 }
 
@@ -4241,7 +4256,8 @@ static int sctp_getsockopt_local_addrs(struct sock *sk, int len,
        rwlock_t *addr_lock;
        int err = 0;
        size_t space_left;
-       int bytes_copied;
+       int bytes_copied = 0;
+       void *addrs;
 
        if (len <= sizeof(struct sctp_getaddrs))
                return -EINVAL;
@@ -4269,6 +4285,9 @@ static int sctp_getsockopt_local_addrs(struct sock *sk, int len,
        to = optval + offsetof(struct sctp_getaddrs,addrs);
        space_left = len - sizeof(struct sctp_getaddrs) -
                         offsetof(struct sctp_getaddrs,addrs);
+       addrs = kmalloc(space_left, GFP_KERNEL);
+       if (!addrs)
+               return -ENOMEM;
 
        sctp_read_lock(addr_lock);
 
@@ -4279,11 +4298,11 @@ static int sctp_getsockopt_local_addrs(struct sock *sk, int len,
                addr = list_entry(bp->address_list.next,
                                  struct sctp_sockaddr_entry, list);
                if (sctp_is_any(&addr->a)) {
-                       cnt = sctp_copy_laddrs_to_user(sk, bp->port,
-                                                      &to, space_left);
+                       cnt = sctp_copy_laddrs(sk, bp->port, addrs,
+                                               space_left, &bytes_copied);
                        if (cnt < 0) {
                                err = cnt;
-                               goto unlock;
+                               goto error;
                        }
                        goto copy_getaddrs;
                }
@@ -4294,26 +4313,31 @@ static int sctp_getsockopt_local_addrs(struct sock *sk, int len,
                memcpy(&temp, &addr->a, sizeof(temp));
                sctp_get_pf_specific(sk->sk_family)->addr_v4map(sp, &temp);
                addrlen = sctp_get_af_specific(temp.sa.sa_family)->sockaddr_len;
-               if(space_left < addrlen)
-                       return -ENOMEM; /*fixme: right error?*/
-               if (copy_to_user(to, &temp, addrlen)) {
-                       err = -EFAULT;
-                       goto unlock;
+               if (space_left < addrlen) {
+                       err =  -ENOMEM; /*fixme: right error?*/
+                       goto error;
                }
+               memcpy(addrs, &temp, addrlen);
                to += addrlen;
+               bytes_copied += addrlen;
                cnt ++;
                space_left -= addrlen;
        }
 
 copy_getaddrs:
+       sctp_read_unlock(addr_lock);
+
+       if (copy_to_user(to, addrs, bytes_copied)) {
+               err = -EFAULT;
+               goto error;
+       }
        if (put_user(cnt, &((struct sctp_getaddrs __user *)optval)->addr_num))
                return -EFAULT;
-       bytes_copied = ((char __user *)to) - optval;
        if (put_user(bytes_copied, optlen))
                return -EFAULT;
 
-unlock:
-       sctp_read_unlock(addr_lock);
+error:
+       kfree(addrs);
        return err;
 }