[SCTP] draft07 API changes: sctp_bindx() now takes a packed array of

sockaddr_in/sockaddr_in6 structures instead of an array of
sockaddr_storage structures.

include/net/sctp/sctp.h

@@ -406,6 +406,12 @@ static inline struct list_head *sctp_list_dequeue(struct list_head *list)
 	return result;
 }
 
+/* Tests if the list has one and only one entry. */
+static inline int sctp_list_single_entry(struct list_head *head)
+{
+	return ((head->next != head) && (head->next == head->prev));
+}
+
 /* Calculate the size (in bytes) occupied by the data of an iovec.  */
 static inline size_t get_user_iov_size(struct iovec *iov, int iovlen)
 {

net/sctp/socket.c

@@ -95,8 +95,8 @@ static int sctp_wait_for_connect(struct sctp_association *, long *timeo_p);
 static int sctp_wait_for_accept(struct sock *sk, long timeo);
 static void sctp_wait_for_close(struct sock *sk, long timeo);
 static inline int sctp_verify_addr(struct sock *, union sctp_addr *, int);
-static int sctp_bindx_add(struct sock *, struct sockaddr_storage *, int);
-static int sctp_bindx_rem(struct sock *, struct sockaddr_storage *, int);
+static int sctp_bindx_add(struct sock *, struct sockaddr *, int);
+static int sctp_bindx_rem(struct sock *, struct sockaddr *, int);
 static int sctp_do_bind(struct sock *, union sctp_addr *, int);
 static int sctp_autobind(struct sock *sk);
 static void sctp_sock_migrate(struct sock *, struct sock *,
@@ -172,10 +172,7 @@ struct sctp_transport *sctp_addr_id2transport(struct sock *sk,
  *   sd      - the socket descriptor returned by socket().
  *   addr    - the address structure (struct sockaddr_in or struct
  *             sockaddr_in6 [RFC 2553]),
- *   addrlen - the size of the address structure.
- *
- * The caller should use struct sockaddr_storage described in RFC 2553
- * to represent addr for portability reason.
+ *   addr_len - the size of the address structure.
  */
 int sctp_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 {
@@ -300,112 +297,6 @@ SCTP_STATIC int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
 	return ret;
 }
 
-/* API 8.1 sctp_bindx()
- *
- * The syntax of sctp_bindx() is,
- *
- *   ret = sctp_bindx(int sd,
- *                    struct sockaddr_storage *addrs,
- * 		      int addrcnt,
- * 		      int flags);
- *
- * If sd is an IPv4 socket, the addresses passed must be IPv4 addresses.
- * If the sd is an IPv6 socket, the addresses passed can either be IPv4
- * or IPv6 addresses.
- *
- * A single address may be specified as INADDR_ANY or IPV6_ADDR_ANY, see
- * section 3.1.2 for this usage.
- *
- * addrs is a pointer to an array of one or more socket addresses.  Each
- * address is contained in a struct sockaddr_storage, so each address is
- * fixed length. The caller specifies the number of addresses in the
- * array with addrcnt.
- *
- * On success, sctp_bindx() returns 0. On failure, sctp_bindx() returns -1,
- * and sets errno to the appropriate error code. [ Editor's note: need
- * to fill in all error code? ]
- *
- * For SCTP, the port given in each socket address must be the same, or
- * sctp_bindx() will fail, setting errno to EINVAL .
- *
- * The flags parameter is formed from the bitwise OR of zero or
- * more of the following currently defined flags:
- *
- *     SCTP_BINDX_ADD_ADDR
- *     SCTP_BINDX_REM_ADDR
- *
- * SCTP_BIND_ADD_ADDR directs SCTP to add the given addresses to the
- * association, and SCTP_BIND_REM_ADDR directs SCTP to remove the given
- * addresses from the association. The two flags are mutually exclusive;
- * if both are given, sctp_bindx() will fail with EINVAL.  A caller may not
- * remove all addresses from an association; sctp_bindx() will reject such
- * an attempt with EINVAL.
- *
- * An application can use sctp_bindx(SCTP_BINDX_ADD_ADDR) to associate
- * additional addresses with an endpoint after calling bind().  Or use
- * sctp_bindx(SCTP_BINDX_REM_ADDR) to remove some addresses a listening
- * socket is associated with so that no new association accepted will be
- * associated with those addresses.
- *
- * SCTP_BIND_ADD_ADDR is defined as 0, so that it becomes the default
- * behavior for sctp_bindx() when no flags are given.
- *
- * Adding and removing addresses from a connected association is optional
- * functionality. Implementations that do not support this functionality
- * should return EOPNOTSUPP.
- *
- * NOTE: This could be integrated into sctp_setsockopt_bindx(),
- * but keeping it this way makes it easier if sometime sys_bindx is
- * added.
- */
-
-/* Unprotected by locks. Call only with socket lock sk->sk_lock held! See
- * sctp_bindx() for a lock-protected call.
- */
-
-static int __sctp_bindx(struct sock *sk, struct sockaddr_storage *addrs,
-			int addrcnt, int flags)
-{
-	int retval = 0;
-
-	SCTP_DEBUG_PRINTK("__sctp_bindx(sk: %p, addrs: %p, addrcnt: %d, "
-			  "flags: %s)\n", sk, addrs, addrcnt,
-			  (SCTP_BINDX_ADD_ADDR == flags) ? "ADD" :
-			  ((SCTP_BINDX_REM_ADDR == flags) ? "REM" : "BOGUS"));
-
-	switch (flags) {
-	case SCTP_BINDX_ADD_ADDR:
-		retval = sctp_bindx_add(sk, addrs, addrcnt);
-		break;
-
-	case SCTP_BINDX_REM_ADDR:
-		retval = sctp_bindx_rem(sk, addrs, addrcnt);
-		break;
-
-	default:
-		retval = -EINVAL;
-		break;
-        };
-
-	return retval;
-}
-
-/* BINDX with locks.
- *
- * NOTE: Currently unused at all ...
- */
-int sctp_bindx(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt,
-	       int flags)
-{
-	int retval;
-
-	sctp_lock_sock(sk);
-	retval = __sctp_bindx(sk, addrs, addrcnt, flags);
-	sctp_release_sock(sk);
-
-	return retval;
-}
-
 /* Add a list of addresses as bind addresses to local endpoint or
  * association.
  *
@@ -416,41 +307,39 @@ int sctp_bindx(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt,
  * If any of them fails, then the operation will be reversed and the
  * ones that were added will be removed.
  *
- * Only __sctp_bindx() is supposed to call this function.
+ * Only sctp_setsockopt_bindx() is supposed to call this function.
  */
-int sctp_bindx_add(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
+int sctp_bindx_add(struct sock *sk, struct sockaddr *addrs, int addrcnt)
 {
 	int cnt;
 	int retval = 0;
-	int addr_len;
+	void *addr_buf;
+	struct sockaddr *sa_addr;
+	struct sctp_af *af;
 
 	SCTP_DEBUG_PRINTK("sctp_bindx_add (sk: %p, addrs: %p, addrcnt: %d)\n",
 			  sk, addrs, addrcnt);
 
+	addr_buf = addrs;
 	for (cnt = 0; cnt < addrcnt; cnt++) {
 		/* The list may contain either IPv4 or IPv6 address;
 		 * determine the address length for walking thru the list.
 		 */
-		switch (((struct sockaddr *)&addrs[cnt])->sa_family) {
-		case AF_INET:
-			addr_len = sizeof(struct sockaddr_in);
-			break;
-
-		case AF_INET6:
-			addr_len = sizeof(struct sockaddr_in6);
-			break;
-
-		default:
+		sa_addr = (struct sockaddr *)addr_buf;
+		af = sctp_get_af_specific(sa_addr->sa_family);
+		if (!af) {
 			retval = -EINVAL;
 			goto err_bindx_add;
-		};
+		}
 
-		retval = sctp_do_bind(sk, (union sctp_addr *)&addrs[cnt],
-				      addr_len);
+		retval = sctp_do_bind(sk, (union sctp_addr *)sa_addr, 
+				      af->sockaddr_len);
+
+		addr_buf += af->sockaddr_len;
 
 err_bindx_add:
 		if (retval < 0) {
-			/* Failed. Cleanup the ones that has been added */
+			/* Failed. Cleanup the ones that have been added */
 			if (cnt > 0)
 				sctp_bindx_rem(sk, addrs, cnt);
 			return retval;
@@ -473,9 +362,9 @@ int sctp_bindx_add(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
  * At least one address has to be left; if only one address is
  * available, the operation will return -EBUSY.
  *
- * Only __sctp_bindx() is supposed to call this function.
+ * Only sctp_setsockopt_bindx() is supposed to call this function.
  */
-int sctp_bindx_rem(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
+int sctp_bindx_rem(struct sock *sk, struct sockaddr *addrs, int addrcnt)
 {
 	struct sctp_opt *sp = sctp_sk(sk);
 	struct sctp_endpoint *ep = sp->ep;
@@ -483,50 +372,41 @@ int sctp_bindx_rem(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
 	struct sctp_bind_addr *bp = &ep->base.bind_addr;
 	int retval = 0;
 	union sctp_addr saveaddr;
+	void *addr_buf;
+	struct sockaddr *sa_addr;
+	struct sctp_af *af;
 
 	SCTP_DEBUG_PRINTK("sctp_bindx_rem (sk: %p, addrs: %p, addrcnt: %d)\n",
 			  sk, addrs, addrcnt);
 
+	addr_buf = addrs;
 	for (cnt = 0; cnt < addrcnt; cnt++) {
-		/* If there is only one bind address, there is nothing more
-		 * to be removed (we need at least one address here).
+		/* If the bind address list is empty or if there is only one
+		 * bind address, there is nothing more to be removed (we need
+		 * at least one address here).
 		 */
-		if (list_empty(&bp->address_list)) {
+		if (list_empty(&bp->address_list) ||
+		    (sctp_list_single_entry(&bp->address_list))) {
 			retval = -EBUSY;
 			goto err_bindx_rem;
 		}
 
 		/* The list may contain either IPv4 or IPv6 address;
-		 * determine the address length for walking thru the list.
+		 * determine the address length to copy the address to
+		 * saveaddr. 
 		 */
-		switch (((struct sockaddr *)&addrs[cnt])->sa_family) {
-		case AF_INET:
-			saveaddr = *((union sctp_addr *)
-				     &addrs[cnt]);
-			saveaddr.v4.sin_port = ntohs(saveaddr.v4.sin_port);
-			/* Verify the port.  */
-			if (saveaddr.v4.sin_port != bp->port) {
-				retval = -EINVAL;
-				goto err_bindx_rem;
-			}
-			break;
-
-		case AF_INET6:
-			saveaddr = *((union sctp_addr *)
-				     &addrs[cnt]);
-			saveaddr.v6.sin6_port =
-				ntohs(saveaddr.v6.sin6_port);
-			/* verify the port */
-			if (saveaddr.v6.sin6_port != bp->port) {
-				retval = -EINVAL;
-				goto err_bindx_rem;
-			}
-			break;
-
-		default:
+		sa_addr = (struct sockaddr *)addr_buf;
+		af = sctp_get_af_specific(sa_addr->sa_family);
+		if (!af) {
 			retval = -EINVAL;
 			goto err_bindx_rem;
-		};
+		}
+		memcpy(&saveaddr, sa_addr, af->sockaddr_len); 
+		saveaddr.v4.sin_port = ntohs(saveaddr.v4.sin_port);
+		if (saveaddr.v4.sin_port != bp->port) {
+			retval = -EINVAL;
+			goto err_bindx_rem;
+		}
 
 		/* FIXME - There is probably a need to check if sk->sk_saddr and
 		 * sk->sk_rcv_addr are currently set to one of the addresses to
@@ -543,6 +423,7 @@ int sctp_bindx_rem(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
 		sctp_write_unlock(&ep->base.addr_lock);
 		sctp_local_bh_enable();
 
+		addr_buf += af->sockaddr_len;
 err_bindx_rem:
 		if (retval < 0) {
 			/* Failed. Add the ones that has been removed back */
@@ -555,18 +436,62 @@ int sctp_bindx_rem(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
 	return retval;
 }
 
-/* Helper for tunneling sys_bindx() requests through sctp_setsockopt()
+/* Helper for tunneling sctp_bindx() requests through sctp_setsockopt()
+ *
+ * API 8.1
+ * int sctp_bindx(int sd, struct sockaddr *addrs, int addrcnt,
+ *                int flags);
+ *
+ * If sd is an IPv4 socket, the addresses passed must be IPv4 addresses.
+ * If the sd is an IPv6 socket, the addresses passed can either be IPv4
+ * or IPv6 addresses.
+ *
+ * A single address may be specified as INADDR_ANY or IN6ADDR_ANY, see
+ * Section 3.1.2 for this usage.
+ *
+ * addrs is a pointer to an array of one or more socket addresses. Each
+ * address is contained in its appropriate structure (i.e. struct
+ * sockaddr_in or struct sockaddr_in6) the family of the address type
+ * must be used to distengish the address length (note that this
+ * representation is termed a "packed array" of addresses). The caller
+ * specifies the number of addresses in the array with addrcnt.
+ *
+ * On success, sctp_bindx() returns 0. On failure, sctp_bindx() returns
+ * -1, and sets errno to the appropriate error code.
+ *
+ * For SCTP, the port given in each socket address must be the same, or
+ * sctp_bindx() will fail, setting errno to EINVAL.
+ *
+ * The flags parameter is formed from the bitwise OR of zero or more of
+ * the following currently defined flags:
+ *
+ * SCTP_BINDX_ADD_ADDR
+ *
+ * SCTP_BINDX_REM_ADDR
+ *
+ * SCTP_BINDX_ADD_ADDR directs SCTP to add the given addresses to the
+ * association, and SCTP_BINDX_REM_ADDR directs SCTP to remove the given
+ * addresses from the association. The two flags are mutually exclusive;
+ * if both are given, sctp_bindx() will fail with EINVAL. A caller may
+ * not remove all addresses from an association; sctp_bindx() will
+ * reject such an attempt with EINVAL.
+ *
+ * An application can use sctp_bindx(SCTP_BINDX_ADD_ADDR) to associate
+ * additional addresses with an endpoint after calling bind().  Or use
+ * sctp_bindx(SCTP_BINDX_REM_ADDR) to remove some addresses a listening
+ * socket is associated with so that no new association accepted will be
+ * associated with those addresses. If the endpoint supports dynamic
+ * address a SCTP_BINDX_REM_ADDR or SCTP_BINDX_ADD_ADDR may cause a
+ * endpoint to send the appropriate message to the peer to change the
+ * peers address lists.
+ *
+ * Adding and removing addresses from a connected association is
+ * optional functionality. Implementations that do not support this
+ * functionality should return EOPNOTSUPP.
  *
  * Basically do nothing but copying the addresses from user to kernel
- * land and invoking sctp_bindx on the sk. This is used for tunneling
- * the sctp_bindx() [sys_bindx()] request through sctp_setsockopt()
- * from userspace.
- *
- * Note I don't use move_addr_to_kernel(): the reason is we would be
- * iterating over an array of struct sockaddr_storage passing always
- * what we know is a good size (sizeof (struct sock...)), so it is
- * pointless. Instead check the whole area for read access and copy
- * it.
+ * land and invoking either sctp_bindx_add() or sctp_bindx_rem() on the sk.
+ * This is used for tunneling the sctp_bindx() request through sctp_setsockopt() * from userspace.
  *
  * We don't use copy_from_user() for optimization: we first do the
  * sanity checks (buffer size -fast- and access check-healthy
@@ -586,38 +511,70 @@ int sctp_bindx_rem(struct sock *sk, struct sockaddr_storage *addrs, int addrcnt)
  *
  * Returns 0 if ok, <0 errno code on error.
  */
-SCTP_STATIC int sctp_setsockopt_bindx(struct sock* sk,
-				      struct sockaddr_storage *addrs,
-				      int addrssize, int op)
+SCTP_STATIC int sctp_setsockopt_bindx(struct sock* sk, struct sockaddr *addrs,
+				      int addrs_size, int op)
 {
-	struct sockaddr_storage *kaddrs;
+	struct sockaddr *kaddrs;
 	int err;
-	size_t addrcnt;
+	int addrcnt = 0;
+	int walk_size = 0;
+	struct sockaddr *sa_addr;
+	void *addr_buf;
+	struct sctp_af *af;
 
-	SCTP_DEBUG_PRINTK("sctp_do_setsocktopt_bindx: sk %p addrs %p"
-			  " addrssize %d opt %d\n", sk, addrs, addrssize, op);
+	SCTP_DEBUG_PRINTK("sctp_setsocktopt_bindx: sk %p addrs %p"
+			  " addrs_size %d opt %d\n", sk, addrs, addrs_size, op);
 
-	/* Do we have an integer number of structs sockaddr_storage?  */
-	if (unlikely(addrssize <= 0 ||
-		     addrssize % sizeof(struct sockaddr_storage) != 0))
+	if (unlikely(addrs_size <= 0))
 		return -EINVAL;
 
 	/* Check the user passed a healthy pointer.  */
-	if (unlikely(!access_ok(VERIFY_READ, addrs, addrssize)))
+	if (unlikely(!access_ok(VERIFY_READ, addrs, addrs_size)))
 		return -EFAULT;
 
 	/* Alloc space for the address array in kernel memory.  */
-	kaddrs = (struct sockaddr_storage *) kmalloc(addrssize, GFP_KERNEL);
+	kaddrs = (struct sockaddr *)kmalloc(addrs_size, GFP_KERNEL);
 	if (unlikely(!kaddrs))
 		return -ENOMEM;
 
-	if (copy_from_user(kaddrs, addrs, addrssize)) {
+	if (__copy_from_user(kaddrs, addrs, addrs_size)) {
 		kfree(kaddrs);
 		return -EFAULT;
 	}
 
-	addrcnt = addrssize / sizeof(struct sockaddr_storage);
-	err = __sctp_bindx(sk, kaddrs, addrcnt, op);   /* Do the work. */
+	/* Walk through the addrs buffer and count the number of addresses. */ 
+	addr_buf = kaddrs;
+	while (walk_size < addrs_size) {
+		sa_addr = (struct sockaddr *)addr_buf;
+		af = sctp_get_af_specific(sa_addr->sa_family);
+
+		/* If the address family is not supported or if this address
+		 * causes the address buffer to overflow return EINVAL.
+		 */ 
+		if (!af || (walk_size + af->sockaddr_len) > addrs_size) {
+			kfree(kaddrs);
+			return -EINVAL;
+		}
+		addrcnt++;
+		addr_buf += af->sockaddr_len;
+		walk_size += af->sockaddr_len;
+	}
+
+	/* Do the work. */
+	switch (op) {
+	case SCTP_BINDX_ADD_ADDR:
+		err = sctp_bindx_add(sk, kaddrs, addrcnt);
+		break;
+
+	case SCTP_BINDX_REM_ADDR:
+		err = sctp_bindx_rem(sk, kaddrs, addrcnt);
+		break;
+
+	default:
+		err = -EINVAL;
+		break;
+        };
+
 	kfree(kaddrs);
 
 	return err;
@@ -1829,16 +1786,14 @@ SCTP_STATIC int sctp_setsockopt(struct sock *sk, int level, int optname,
 	switch (optname) {
 	case SCTP_SOCKOPT_BINDX_ADD:
 		/* 'optlen' is the size of the addresses buffer. */
-		retval = sctp_setsockopt_bindx(sk, (struct sockaddr_storage *)
-					       optval, optlen,
-					       SCTP_BINDX_ADD_ADDR);
+		retval = sctp_setsockopt_bindx(sk, (struct sockaddr *)optval,
+					       optlen, SCTP_BINDX_ADD_ADDR);
 		break;
 
 	case SCTP_SOCKOPT_BINDX_REM:
 		/* 'optlen' is the size of the addresses buffer. */
-		retval = sctp_setsockopt_bindx(sk, (struct sockaddr_storage *)
-					       optval, optlen,
-					       SCTP_BINDX_REM_ADDR);
+		retval = sctp_setsockopt_bindx(sk, (struct sockaddr *)optval,
+					       optlen, SCTP_BINDX_REM_ADDR);
 		break;
 
 	case SCTP_DISABLE_FRAGMENTS: