diff mbox

ieee802154: verify packet size before trying to allocate it

Message ID 1339326619-1753-1-git-send-email-levinsasha928@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Sasha Levin June 10, 2012, 11:10 a.m. UTC
Currently when sending data over datagram, the send function will attempt to
allocate any size passed on from the userspace.

We should make sure that this size is checked and limited. The maximum size
of an IP packet seemed like the safest limit here.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---

Change in v2:
 - Limit by maximum size the protocol supports.

 net/ieee802154/dgram.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Comments

Alan Cox June 10, 2012, 11:24 a.m. UTC | #1
On Sun, 10 Jun 2012 13:10:19 +0200
Sasha Levin <levinsasha928@gmail.com> wrote:

> Currently when sending data over datagram, the send function will attempt to
> allocate any size passed on from the userspace.
> 
> We should make sure that this size is checked and limited. The maximum size
> of an IP packet seemed like the safest limit here.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> 
> Change in v2:
>  - Limit by maximum size the protocol supports.
> 
>  net/ieee802154/dgram.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
> index 6fbb2ad..628498c 100644
> --- a/net/ieee802154/dgram.c
> +++ b/net/ieee802154/dgram.c
> @@ -232,6 +232,11 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
>  
>  	hlen = LL_RESERVED_SPACE(dev);
>  	tlen = dev->needed_tailroom;
> +	if (hlen + tlen + size > IEEE802154_MTU) {
> +		err = -EMSGSIZE;
> +		goto out;

What stops an overflow at this point. We'll then pass a small value to
sock_alloc_send_skb/sock_alloc_send_pskb and copy a large number of bytes
into it.

This does seem to be already broken, and not fixed by the patch ?

Alan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Ceuleers June 10, 2012, 12:55 p.m. UTC | #2
On 06/10/2012 01:10 PM, Sasha Levin wrote:
> Currently when sending data over datagram, the send function will attempt to
> allocate any size passed on from the userspace.
> 
> We should make sure that this size is checked and limited. The maximum size
> of an IP packet seemed like the safest limit here.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

As I understand it this issue was present in the original code that was
introduced in 2.6.31 RC1. Should this therefore be submitted to stable
(in which case David will do so)?

Commit ID 9ec7671603573ede31207eb5b0b3e1aa211b2854

Thanks, Jan
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 11, 2012, 3:04 a.m. UTC | #3
From: Sasha Levin <levinsasha928@gmail.com>
Date: Sun, 10 Jun 2012 13:10:19 +0200

> Currently when sending data over datagram, the send function will attempt to
> allocate any size passed on from the userspace.
> 
> We should make sure that this size is checked and limited. The maximum size
> of an IP packet seemed like the safest limit here.
> 
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

Why not limit to the device MTU?  That's exactly what I suggested
to you.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sasha Levin June 11, 2012, 8:18 a.m. UTC | #4
On Sun, 2012-06-10 at 20:04 -0700, David Miller wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
> Date: Sun, 10 Jun 2012 13:10:19 +0200
> 
> > Currently when sending data over datagram, the send function will attempt to
> > allocate any size passed on from the userspace.
> > 
> > We should make sure that this size is checked and limited. The maximum size
> > of an IP packet seemed like the safest limit here.
> > 
> > Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> 
> Why not limit to the device MTU?  That's exactly what I suggested
> to you.

That's what I ended up doing in the reply to this mail.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 6fbb2ad..628498c 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -232,6 +232,11 @@  static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
 
 	hlen = LL_RESERVED_SPACE(dev);
 	tlen = dev->needed_tailroom;
+	if (hlen + tlen + size > IEEE802154_MTU) {
+		err = -EMSGSIZE;
+		goto out;
+	}
+		
 	skb = sock_alloc_send_skb(sk, hlen + tlen + size,
 			msg->msg_flags & MSG_DONTWAIT,
 			&err);