From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030732AbXAZDbb (ORCPT ); Thu, 25 Jan 2007 22:31:31 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1030730AbXAZDbb (ORCPT ); Thu, 25 Jan 2007 22:31:31 -0500 Received: from 74-93-104-97-Washington.hfc.comcastbusiness.net ([74.93.104.97]:48727 "EHLO sunset.davemloft.net" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1030720AbXAZDba (ORCPT ); Thu, 25 Jan 2007 22:31:30 -0500 Date: Thu, 25 Jan 2007 19:31:28 -0800 (PST) Message-Id: <20070125.193128.39155864.davem@davemloft.net> To: linux-kernel@vger.kernel.org, akpm@osdl.org Cc: deweerdt@free.fr, netdev@vger.kernel.org, frederik.deweerdt@gmail.com, shemminger@osdl.org Subject: Re: + oops-in-drivers-net-shaperc.patch added to -mm tree From: David Miller In-Reply-To: <200701250354.l0P3spES005128@shell0.pdx.osdl.net> References: <200701250354.l0P3spES005128@shell0.pdx.osdl.net> X-Mailer: Mew version 5.1.52 on Emacs 21.4 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org From: akpm@osdl.org Date: Wed, 24 Jan 2007 19:54:51 -0800 > Hi, > > The following code: > [...] > > Causes the following oops: > ... > [ 66.355188] [] error_code+0x7c/0x84 > [ 66.355192] [] packet_sendmsg+0x147/0x201 [af_packet] > [ 66.355199] [] sock_sendmsg+0xf9/0x116 > [ 66.355204] [] sys_sendto+0xbf/0xe0 > [ 66.355208] [] sys_socketcall+0x1aa/0x277 > [ 66.355212] [] 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 > Cc: "David S. Miller" > Cc: Stephen Hemminger > Signed-off-by: Andrew Morton 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 */