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 a9663b4..92eae0e 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -404,19 +404,6 @@
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 1eac3d0..de313de 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -221,8 +221,7 @@
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 4f11f58..17b5092 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -806,38 +806,26 @@
/* 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;
+ void *payload = NULL;
+ int err;
- if (msg) {
- iov = msg->msg_iov;
- iovlen = msg->msg_iovlen;
- paylen = get_user_iov_size(iov, iovlen);
- }
-
- 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 ead3f1b..5b5ae79 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -4031,18 +4031,12 @@
* 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 @@
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_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 54722e6..fde3f55 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1520,8 +1520,16 @@
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;
}