From patchwork Wed Sep 2 08:58:52 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ralf Baechle X-Patchwork-Id: 32818 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@bilbo.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from ozlabs.org (ozlabs.org [203.10.76.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx.ozlabs.org", Issuer "CA Cert Signing Authority" (verified OK)) by bilbo.ozlabs.org (Postfix) with ESMTPS id 3F8D8B7BB4 for ; Wed, 2 Sep 2009 18:58:20 +1000 (EST) Received: by ozlabs.org (Postfix) id 2EA9BDDD0B; Wed, 2 Sep 2009 18:58:20 +1000 (EST) Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by ozlabs.org (Postfix) with ESMTP id C0959DDD01 for ; Wed, 2 Sep 2009 18:58:19 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751141AbZIBI6G (ORCPT ); Wed, 2 Sep 2009 04:58:06 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751192AbZIBI6F (ORCPT ); Wed, 2 Sep 2009 04:58:05 -0400 Received: from h5.dl5rb.org.uk ([81.2.74.5]:39950 "EHLO h5.dl5rb.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750872AbZIBI6E (ORCPT ); Wed, 2 Sep 2009 04:58:04 -0400 Received: from h5.dl5rb.org.uk (localhost.localdomain [127.0.0.1]) by h5.dl5rb.org.uk (8.14.3/8.14.3) with ESMTP id n828wuDt006949; Wed, 2 Sep 2009 09:58:57 +0100 Received: (from ralf@localhost) by h5.dl5rb.org.uk (8.14.3/8.14.3/Submit) id n828wt6h006947; Wed, 2 Sep 2009 09:58:55 +0100 Date: Wed, 2 Sep 2009 09:58:52 +0100 From: Ralf Baechle To: "David S. Miller" , netdev@vger.kernel.org Cc: linux-hams@vger.kernel.org, Thomas Osterried , Jann Traschewski Subject: [PATCH] NET: Fix possible corruption in bpqether driver Message-ID: <20090902085841.GA5910@linux-mips.org> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.19 (2009-01-05) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The bpq ether driver is modifying the data art of the skb by first dropping the KISS byte (a command byte for the radio) then prepending the length + 4 of the remaining AX.25 packet to be transmitted as a little endian 16-bit number. If the high byte of the length has a different value than the dropped KISS byte users of clones of the skb may observe this as corruption. This was observed with by running listen(8) -a which uses a packet socket which clones transmit packets. The corruption will then typically be displayed for as a KISS "TX Delay" command for AX.25 packets in the range of 252..508 bytes or any other KISS command for yet larger packets. Fixed by using skb_cow to create a private copy should the skb be cloned. Using skb_cow also allows us to cleanup the old logic to ensure sufficient headroom in the skb. While at it, replace a return of 0 from bpq_xmit with the proper constant NETDEV_TX_OK which is now being used everywhere else in this function. Affected: all 2.2, 2.4 and 2.6 kernels. Signed-off-by: Ralf Baechle Reported-by: Jann Traschewski drivers/net/hamradio/bpqether.c | 29 ++++++++++++----------------- 1 files changed, 12 insertions(+), 17 deletions(-) --- 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 --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c index abcd19a..184a528 100644 --- a/drivers/net/hamradio/bpqether.c +++ b/drivers/net/hamradio/bpqether.c @@ -249,7 +249,6 @@ drop: */ static int bpq_xmit(struct sk_buff *skb, struct net_device *dev) { - struct sk_buff *newskb; unsigned char *ptr; struct bpqdev *bpq; int size; @@ -263,28 +262,23 @@ static int bpq_xmit(struct sk_buff *skb, struct net_device *dev) return NETDEV_TX_OK; } - skb_pull(skb, 1); + skb_pull(skb, 1); /* Drop KISS byte */ size = skb->len; /* - * The AX.25 code leaves enough room for the ethernet header, but - * sendto() does not. + * We're about to mess with the skb which may still shared with the + * generic networking code so unshare and ensure it's got enough + * space for the BPQ headers. */ - if (skb_headroom(skb) < AX25_BPQ_HEADER_LEN) { /* Ough! */ - if ((newskb = skb_realloc_headroom(skb, AX25_BPQ_HEADER_LEN)) == NULL) { - printk(KERN_WARNING "bpqether: out of memory\n"); - kfree_skb(skb); - return NETDEV_TX_OK; - } - - if (skb->sk != NULL) - skb_set_owner_w(newskb, skb->sk); - + if (skb_cow(skb, AX25_BPQ_HEADER_LEN)) { + if (net_ratelimit()) + pr_err("bpqether: out of memory\n"); kfree_skb(skb); - skb = newskb; + + return NETDEV_TX_OK; } - ptr = skb_push(skb, 2); + ptr = skb_push(skb, 2); /* Make space for length */ *ptr++ = (size + 5) % 256; *ptr++ = (size + 5) / 256; @@ -305,7 +299,8 @@ static int bpq_xmit(struct sk_buff *skb, struct net_device *dev) dev_queue_xmit(skb); netif_wake_queue(dev); - return 0; + + return NETDEV_TX_OK; } /*