Fix sctp privilege elevation (CVE-2006-3745)

sctp_make_abort_user() now takes the msg_len along with the msg
so that we don't have to recalculate the bytes in iovec.
It also uses memcpy_fromiovec() so that we don't go beyond the
length allocated.

It is good to have this fix even if verify_iovec() is fixed to
return error on overflow.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index a9663b49ea54ba494ec2ca523e309d69008cffeb..92eae0e0f3f152d54534fedd0633ff3d44f34ba3 100644 (file)
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -404,19 +404,6 @@ 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)
-{
-       size_t retval = 0;
-
-       for (; iovlen > 0; --iovlen) {
-               retval += iov->iov_len;
-               iov++;
-       }
-
-       return retval;
-}
-
 /* Generate a random jitter in the range of -50% ~ +50% of input RTO. */
 static inline __s32 sctp_jitter(__u32 rto)
 {

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index 1eac3d0eb7a9915c21db0592f5b231c00803052f..de313de4fefe53733a055f1dea20aef62779e1dc 100644 (file)
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -221,8 +221,7 @@ struct sctp_chunk *sctp_make_abort_no_data(const struct sctp_association *,
                                      const struct sctp_chunk *,
                                      __u32 tsn);
 struct sctp_chunk *sctp_make_abort_user(const struct sctp_association *,
-                                  const struct sctp_chunk *,
-                                  const struct msghdr *);
+                                       const struct msghdr *, size_t msg_len);
 struct sctp_chunk *sctp_make_abort_violation(const struct sctp_association *,
                                   const struct sctp_chunk *,
                                   const __u8 *,

diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 4f11f5858209b22d67b2237a3957fb0f6cab1ff5..17b509282cf2067f6052374536ddb36c9fbf404a 100644 (file)
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -806,38 +806,26 @@ no_mem:
 
 /* Helper to create ABORT with a SCTP_ERROR_USER_ABORT error.  */
 struct sctp_chunk *sctp_make_abort_user(const struct sctp_association *asoc,
-                                  const struct sctp_chunk *chunk,
-                                  const struct msghdr *msg)
+                                       const struct msghdr *msg,
+                                       size_t paylen)
 {
        struct sctp_chunk *retval;
-       void *payload = NULL, *payoff;
-       size_t paylen = 0;
-       struct iovec *iov = NULL;
-       int iovlen = 0;
-
-       if (msg) {
-               iov = msg->msg_iov;
-               iovlen = msg->msg_iovlen;
-               paylen = get_user_iov_size(iov, iovlen);
-       }
+       void *payload = NULL;
+       int err;
 
-       retval = sctp_make_abort(asoc, chunk, sizeof(sctp_errhdr_t) + paylen);
+       retval = sctp_make_abort(asoc, NULL, sizeof(sctp_errhdr_t) + paylen);
        if (!retval)
                goto err_chunk;
 
        if (paylen) {
                /* Put the msg_iov together into payload.  */
-               payload = kmalloc(paylen, GFP_ATOMIC);
+               payload = kmalloc(paylen, GFP_KERNEL);
                if (!payload)
                        goto err_payload;
-               payoff = payload;
 
-               for (; iovlen > 0; --iovlen) {
-                       if (copy_from_user(payoff, iov->iov_base,iov->iov_len))
-                               goto err_copy;
-                       payoff += iov->iov_len;
-                       iov++;
-               }
+               err = memcpy_fromiovec(payload, msg->msg_iov, paylen);
+               if (err < 0)
+                       goto err_copy;
        }
 
        sctp_init_cause(retval, SCTP_ERROR_USER_ABORT, payload, paylen);

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index ead3f1b0ea3dad20e0844cc3b21be2ac2d75fa9e..5b5ae79583223244ef2c85ed8aa5d331b0c829dd 100644 (file)
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4031,18 +4031,12 @@ sctp_disposition_t sctp_sf_do_9_1_prm_abort(
         * from its upper layer, but retransmits data to the far end
         * if necessary to fill gaps.
         */
-       struct msghdr *msg = arg;
-       struct sctp_chunk *abort;
+       struct sctp_chunk *abort = arg;
        sctp_disposition_t retval;
 
        retval = SCTP_DISPOSITION_CONSUME;
 
-       /* Generate ABORT chunk to send the peer.  */
-       abort = sctp_make_abort_user(asoc, NULL, msg);
-       if (!abort)
-               retval = SCTP_DISPOSITION_NOMEM;
-       else
-               sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
+       sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
 
        /* Even if we can't send the ABORT due to low memory delete the
         * TCB.  This is a departure from our typical NOMEM handling.
@@ -4166,8 +4160,7 @@ sctp_disposition_t sctp_sf_cookie_wait_prm_abort(
        void *arg,
        sctp_cmd_seq_t *commands)
 {
-       struct msghdr *msg = arg;
-       struct sctp_chunk *abort;
+       struct sctp_chunk *abort = arg;
        sctp_disposition_t retval;
 
        /* Stop T1-init timer */
@@ -4175,12 +4168,7 @@ sctp_disposition_t sctp_sf_cookie_wait_prm_abort(
                        SCTP_TO(SCTP_EVENT_TIMEOUT_T1_INIT));
        retval = SCTP_DISPOSITION_CONSUME;
 
-       /* Generate ABORT chunk to send the peer */
-       abort = sctp_make_abort_user(asoc, NULL, msg);
-       if (!abort)
-               retval = SCTP_DISPOSITION_NOMEM;
-       else
-               sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
+       sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(abort));
 
        sctp_add_cmd_sf(commands, SCTP_CMD_NEW_STATE,
                        SCTP_STATE(SCTP_STATE_CLOSED));

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 54722e622e6dbf68d44be8f82cdb01f72ab7b558..fde3f55bfd4b43da353202ca25a552ad54a70940 100644 (file)
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1520,8 +1520,16 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk,
                        goto out_unlock;
                }
                if (sinfo_flags & SCTP_ABORT) {
+                       struct sctp_chunk *chunk;
+
+                       chunk = sctp_make_abort_user(asoc, msg, msg_len);
+                       if (!chunk) {
+                               err = -ENOMEM;
+                               goto out_unlock;
+                       }
+
                        SCTP_DEBUG_PRINTK("Aborting association: %p\n", asoc);
-                       sctp_primitive_ABORT(asoc, msg);
+                       sctp_primitive_ABORT(asoc, chunk);
                        err = 0;
                        goto out_unlock;
                }