[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20070125.193128.39155864.davem@davemloft.net>
Date: Thu, 25 Jan 2007 19:31:28 -0800 (PST)
From: David Miller <davem@...emloft.net>
To: linux-kernel@...r.kernel.org, akpm@...l.org
Cc: deweerdt@...e.fr, netdev@...r.kernel.org,
frederik.deweerdt@...il.com, shemminger@...l.org
Subject: Re: + oops-in-drivers-net-shaperc.patch added to -mm tree
From: akpm@...l.org
Date: Wed, 24 Jan 2007 19:54:51 -0800
> Hi,
>
> The following code:
> [...]
>
> Causes the following oops:
>
...
> [ 66.355188] [<c0396c74>] error_code+0x7c/0x84
> [ 66.355192] [<f8adaf03>] packet_sendmsg+0x147/0x201 [af_packet]
> [ 66.355199] [<c030e1c5>] sock_sendmsg+0xf9/0x116
> [ 66.355204] [<c030eb54>] sys_sendto+0xbf/0xe0
> [ 66.355208] [<c030f494>] sys_socketcall+0x1aa/0x277
> [ 66.355212] [<c01041ea>] sysenter_past_esp+0x5f/0x99
> [ 66.355216] =======================
> [ 66.355218] Code: Bad EIP value.
> [ 66.355223] EIP: [<00000000>] 0x0 SS:ESP 0068:f6261d70
>
> shaper_header() should check for shaper->dev not being NULL (ie. the
> shaper was actually attached) as in the following patch.
> This happens in mainline too (tested 2.6.19.2).
>
> Signed-off-by: Frederik Deweerdt <frederik.deweerdt@...il.com>
> Cc: "David S. Miller" <davem@...emloft.net>
> Cc: Stephen Hemminger <shemminger@...l.org>
> Signed-off-by: Andrew Morton <akpm@...l.org>
Shaper is actually OK. None of these hardware header callbacks
should be invoked if the device is down. Yet, this is what is
accidently being allowed in the AF_PACKET socket layer.
Shaper makes sure to fail ->open() if shaper->dev is NULL, in order
to prevent this.
But AF_PACKET does it's check of device state too late, after the
dev->header() call. That's the bug.
I'll fix it like this:
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 594c078..6dc01bd 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -359,6 +359,10 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
if (dev == NULL)
goto out_unlock;
+ err = -ENETDOWN;
+ if (!(dev->flags & IFF_UP))
+ goto out_unlock;
+
/*
* You may not queue a frame bigger than the mtu. This is the lowest level
* raw protocol and you must do your own fragmentation at this level.
@@ -407,10 +411,6 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
if (err)
goto out_free;
- err = -ENETDOWN;
- if (!(dev->flags & IFF_UP))
- goto out_free;
-
/*
* Now send it
*/
@@ -738,6 +738,10 @@ static int packet_sendmsg(struct kiocb *iocb, struct socket *sock,
if (sock->type == SOCK_RAW)
reserve = dev->hard_header_len;
+ err = -ENETDOWN;
+ if (!(dev->flags & IFF_UP))
+ goto out_unlock;
+
err = -EMSGSIZE;
if (len > dev->mtu+reserve)
goto out_unlock;
@@ -770,10 +774,6 @@ static int packet_sendmsg(struct kiocb *iocb, struct socket *sock,
skb->dev = dev;
skb->priority = sk->sk_priority;
- err = -ENETDOWN;
- if (!(dev->flags & IFF_UP))
- goto out_free;
-
/*
* Now send it
*/
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists