* [PATCH] ieee802154: verify packet size before trying to allocate it
@ 2012-06-10 11:10 Sasha Levin
2012-06-10 11:24 ` Alan Cox
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sasha Levin @ 2012-06-10 11:10 UTC (permalink / raw)
To: dbaryshkov, slapin, davem
Cc: linux-zigbee-devel, netdev, linux-kernel, Sasha Levin
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;
+ }
+
skb = sock_alloc_send_skb(sk, hlen + tlen + size,
msg->msg_flags & MSG_DONTWAIT,
&err);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-10 11:10 [PATCH] ieee802154: verify packet size before trying to allocate it Sasha Levin
@ 2012-06-10 11:24 ` Alan Cox
2012-06-10 12:16 ` Sasha Levin
2012-06-10 12:55 ` Jan Ceuleers
2012-06-11 3:04 ` David Miller
2 siblings, 1 reply; 8+ messages in thread
From: Alan Cox @ 2012-06-10 11:24 UTC (permalink / raw)
To: Sasha Levin
Cc: dbaryshkov, slapin, davem, linux-zigbee-devel, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-10 11:24 ` Alan Cox
@ 2012-06-10 12:16 ` Sasha Levin
0 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2012-06-10 12:16 UTC (permalink / raw)
To: Alan Cox
Cc: dbaryshkov, slapin, davem, linux-zigbee-devel, netdev, linux-kernel
Hi Alan,
On Sun, 2012-06-10 at 12:24 +0100, Alan Cox wrote:
> On Sun, 10 Jun 2012 13:10:19 +0200
> Sasha Levin <levinsasha928@gmail.com> wrote:
> > + 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
Hm, nothing.
I've added this check to prevent users from being able to allocate huge kernel buffers, and haven't though about the overflow case at all. Thanks for pointing it out.
How about something like this instead:
-----8<-----
From: Sasha Levin <levinsasha928@gmail.com>
Date: Sun, 10 Jun 2012 13:08:03 +0200
Subject: [PATCH] ieee802154: verify packet size before trying to allocate it
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>
---
net/ieee802154/dgram.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 6fbb2ad..b098b9c 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -230,6 +230,12 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
mtu = dev->mtu;
pr_debug("name = %s, mtu = %u\n", dev->name, mtu);
+ if (size > mtu) {
+ pr_debug("size = %Zu, mtu = %u\n", size, mtu);
+ err = -EINVAL;
+ goto out_skb;
+ }
+
hlen = LL_RESERVED_SPACE(dev);
tlen = dev->needed_tailroom;
skb = sock_alloc_send_skb(sk, hlen + tlen + size,
@@ -258,12 +264,6 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
if (err < 0)
goto out_skb;
- if (size > mtu) {
- pr_debug("size = %Zu, mtu = %u\n", size, mtu);
- err = -EINVAL;
- goto out_skb;
- }
-
skb->dev = dev;
skb->sk = sk;
skb->protocol = htons(ETH_P_IEEE802154);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-10 11:10 [PATCH] ieee802154: verify packet size before trying to allocate it Sasha Levin
2012-06-10 11:24 ` Alan Cox
@ 2012-06-10 12:55 ` Jan Ceuleers
2012-06-11 3:04 ` David Miller
2 siblings, 0 replies; 8+ messages in thread
From: Jan Ceuleers @ 2012-06-10 12:55 UTC (permalink / raw)
To: Sasha Levin
Cc: dbaryshkov, slapin, davem, linux-zigbee-devel, netdev, linux-kernel
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-10 11:10 [PATCH] ieee802154: verify packet size before trying to allocate it Sasha Levin
2012-06-10 11:24 ` Alan Cox
2012-06-10 12:55 ` Jan Ceuleers
@ 2012-06-11 3:04 ` David Miller
2012-06-11 8:18 ` Sasha Levin
2 siblings, 1 reply; 8+ messages in thread
From: David Miller @ 2012-06-11 3:04 UTC (permalink / raw)
To: levinsasha928
Cc: dbaryshkov, slapin, linux-zigbee-devel, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-11 3:04 ` David Miller
@ 2012-06-11 8:18 ` Sasha Levin
0 siblings, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2012-06-11 8:18 UTC (permalink / raw)
To: David Miller; +Cc: dbaryshkov, slapin, linux-zigbee-devel, netdev, linux-kernel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ieee802154: verify packet size before trying to allocate it
2012-06-06 21:32 Sasha Levin
@ 2012-06-07 20:10 ` David Miller
0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2012-06-07 20:10 UTC (permalink / raw)
To: levinsasha928
Cc: dbaryshkov, slapin, linux-zigbee-devel, netdev, linux-kernel
From: Sasha Levin <levinsasha928@gmail.com>
Date: Wed, 6 Jun 2012 23:32:02 +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>
This limit is arbitrary, I'm not applying a patch like this.
Use the actual limit, which is either the protocol limit or something
like the device mtu.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ieee802154: verify packet size before trying to allocate it
@ 2012-06-06 21:32 Sasha Levin
2012-06-07 20:10 ` David Miller
0 siblings, 1 reply; 8+ messages in thread
From: Sasha Levin @ 2012-06-06 21:32 UTC (permalink / raw)
To: dbaryshkov-Re5JQEeQqe8AvxtiuMwx3w, slapin-9cOl001CZnBAfugRpC6u6w,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Sasha Levin
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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
net/ieee802154/dgram.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 6fbb2ad..cf5070b 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -232,6 +232,10 @@ static int dgram_sendmsg(struct kiocb *iocb, struct sock *sk,
hlen = LL_RESERVED_SPACE(dev);
tlen = dev->needed_tailroom;
+ if (hlen + tlen + size > USHRT_MAX) {
+ err = -EMSGSIZE;
+ goto out;
+ }
skb = sock_alloc_send_skb(sk, hlen + tlen + size,
msg->msg_flags & MSG_DONTWAIT,
&err);
--
1.7.8.6
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-06-11 8:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-10 11:10 [PATCH] ieee802154: verify packet size before trying to allocate it Sasha Levin
2012-06-10 11:24 ` Alan Cox
2012-06-10 12:16 ` Sasha Levin
2012-06-10 12:55 ` Jan Ceuleers
2012-06-11 3:04 ` David Miller
2012-06-11 8:18 ` Sasha Levin
-- strict thread matches above, loose matches on Subject: below --
2012-06-06 21:32 Sasha Levin
2012-06-07 20:10 ` David Miller
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).