netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: torvalds@linux-foundation.org
Cc: drosenberg@vsecurity.com, jon.maloy@ericsson.com,
	allan.stephens@windriver.com, netdev@vger.kernel.org,
	security@kernel.org
Subject: Re: [Security] TIPC security issues
Date: Wed, 27 Oct 2010 10:50:47 -0700 (PDT)	[thread overview]
Message-ID: <20101027.105047.183059900.davem@davemloft.net> (raw)
In-Reply-To: <AANLkTinHVXDJbgJ0zBVRnr8R4sx=NtZCaFgbFC+8DE+q@mail.gmail.com>

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Wed, 27 Oct 2010 10:37:46 -0700

> If you _really_ care deeply, then some packet-oriented protocol can
> just have its own private packet size limit (which would be way less
> than 2GB), and then just look at the total size and say "oh, the total
> size is bigger than my limit, so I'll just error out". Then, the fact
> that verify_iovec() may have truncated the message to 2GB-1 doesn't
> matter at all.
> 
> (Practically speaking, I bet all packet-oriented protocols already
> have a limit that is enforced by simply allocation patterns, so I
> don't think it's actually a problem even now)

This is, as it turns out, effectively what the TIPC socket layer
already does.

Most of the send calls that propagate down to this code adding up the
iov_len lengths gets passed a maximum packet size.

Anyways, here is what I came up with to kill this specific bug in
TIPC:

>From 39dc17049a5ed989bab8997945a048ffddf48387 Mon Sep 17 00:00:00 2001
From: David S. Miller <davem@davemloft.net>
Date: Wed, 27 Oct 2010 10:46:59 -0700
Subject: [PATCH] tipc: Fix iov_len handling in message send path.

Use size_t to add together iov_len's from the iovec, error
out with -EMSGSIZE if total is greater than INT_MAX.

Reported-by: Dan Rosenberg <drosenberg@vsecurity.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/tipc/msg.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ecb532f..b29eb8b 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -76,11 +76,15 @@ void tipc_msg_init(struct tipc_msg *m, u32 user, u32 type,
 
 int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect)
 {
-	int dsz = 0;
+	size_t dsz = 0;
 	int i;
 
 	for (i = 0; i < num_sect; i++)
 		dsz += msg_sect[i].iov_len;
+
+	if (dsz > INT_MAX)
+		return -EMSGSIZE;
+
 	return dsz;
 }
 
@@ -93,12 +97,15 @@ int tipc_msg_calc_data_size(struct iovec const *msg_sect, u32 num_sect)
  */
 
 int tipc_msg_build(struct tipc_msg *hdr,
-			    struct iovec const *msg_sect, u32 num_sect,
-			    int max_size, int usrmem, struct sk_buff** buf)
+		      struct iovec const *msg_sect, u32 num_sect,
+		      int max_size, int usrmem, struct sk_buff** buf)
 {
 	int dsz, sz, hsz, pos, res, cnt;
 
 	dsz = tipc_msg_calc_data_size(msg_sect, num_sect);
+	if (dsz < 0)
+		return dsz;
+
 	if (unlikely(dsz > TIPC_MAX_USER_MSG_SIZE)) {
 		*buf = NULL;
 		return -EINVAL;
-- 
1.7.3.2


  reply	other threads:[~2010-10-27 17:50 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-10-21 23:45 TIPC security issues Dan Rosenberg
2010-10-22  0:31 ` [Security] " Linus Torvalds
2010-10-25  2:14   ` David Miller
2010-10-25  3:42     ` Linus Torvalds
2010-10-25  5:28       ` David Miller
2010-10-27 17:29   ` David Miller
2010-10-27 17:37     ` Linus Torvalds
2010-10-27 17:50       ` David Miller [this message]
2010-10-27 18:26         ` Dan Rosenberg
2010-10-27 18:34           ` David Miller
2010-10-27 18:51           ` Linus Torvalds
2010-10-27 19:27             ` David Miller
2010-10-28 15:32               ` Linus Torvalds
2010-10-28 18:45                 ` Andy Grover
2010-10-28 18:49                   ` David Miller
2010-10-27 18:27         ` Paul Gortmaker
2010-10-27 18:35           ` David Miller
2010-10-27 19:00             ` Paul Gortmaker
2010-10-28 19:51         ` Paul Gortmaker
2010-10-22 13:49 ` Jon Maloy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20101027.105047.183059900.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=allan.stephens@windriver.com \
    --cc=drosenberg@vsecurity.com \
    --cc=jon.maloy@ericsson.com \
    --cc=netdev@vger.kernel.org \
    --cc=security@kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).