From patchwork Wed Jan 20 15:48:34 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 570753 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 25A661401CA for ; Thu, 21 Jan 2016 02:48:57 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=X7hhABVQ; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934119AbcATPsi (ORCPT ); Wed, 20 Jan 2016 10:48:38 -0500 Received: from mail-pf0-f179.google.com ([209.85.192.179]:33251 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756060AbcATPsh (ORCPT ); Wed, 20 Jan 2016 10:48:37 -0500 Received: by mail-pf0-f179.google.com with SMTP id e65so6622003pfe.0; Wed, 20 Jan 2016 07:48:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:subject:from:to:cc:date:in-reply-to:references :content-type:mime-version:content-transfer-encoding; bh=ND6P+ec+SEinZ6sljFouImSt/8aW7vfHy/vGAVkcE64=; b=X7hhABVQi1FR2dpkF2L6aR02qCa9Am1JqenCfGDutBeo6UrH64uHNxQMW7wdPX+Sfa DXlAhQmO/m1OQ6jYlUM/+rc/5NzqGp6iZpUVJX7Wz3055Qlx5CdRqznBWt2YGNVfhXnv PccuSnlelZfnDix2lukNynC5nGBUw8UBaPkiZ+gOJiclWJSuW0zBYFmVtsW88rCm3PI+ UMw1Qa00qacfjoucPA7IcC5JOl51i2fSkDDpz3Fzu+s1fHqbpL+h06YMXLlYEBn8nZ08 OjOZPQd3Z2o89FntCsE51uZPeRdRgYwL68TCOJLxuJ0jvdhOmXEkN9CvaiBcJKzEWUKc 3pmw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-type:mime-version:content-transfer-encoding; bh=ND6P+ec+SEinZ6sljFouImSt/8aW7vfHy/vGAVkcE64=; b=OCTyUR5P8hpW3musL/5RKVdg8+haFfPiqRoPmvEDCpMh4/CRNi1A4JfE5rGXOO+OGv 36vSxJ4djYsinQ74rlvjtIvJf61juOihnAUHLdL9rVaLNsPmB7fBLYvdA2lWILjKmZvD sn7X1ew+9mUoeKRpy21mpSU9St9kS1VOtEDsVq9DvWK9yWziiOWV0HTnVvX7YkHiLpjC cWJD0aGiFMHCE43EhFQkPOv3hWdc4vmqCkQF4Avzh6Jdd8QojpSdXdPogF33iLHD6fvl WKdTz/OUSs3zPTP/mTHiiXxP8OrvlQQSsrK8dKCmzyrUL1vUAlIKiNwap3sFtVLKnJzY 4yZQ== X-Gm-Message-State: ALoCoQmMWitnu52Y4RM2ROsAAiEdwmiz+v88H4o7pmBLQ68nZWy63s6nwbaxVIxe0+wDzQxBO5YZNVPL0udTiAh//btFzPDFIg== X-Received: by 10.98.65.9 with SMTP id o9mr52884820pfa.114.1453304916437; Wed, 20 Jan 2016 07:48:36 -0800 (PST) Received: from ?IPv6:2620:0:1000:3e02:1e7:21a7:5394:d4f1? ([2620:0:1000:3e02:1e7:21a7:5394:d4f1]) by smtp.gmail.com with ESMTPSA id n2sm49754455pfj.16.2016.01.20.07.48.34 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 20 Jan 2016 07:48:35 -0800 (PST) Message-ID: <1453304914.1223.325.camel@edumazet-glaptop2.roam.corp.google.com> Subject: Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram From: Eric Dumazet To: Jacob Siverskog Cc: Cong Wang , Eric Dumazet , David Miller , Rainer Weikusat , netdev , Herbert Xu , Konstantin Khlebnikov , Al Viro , LKML Date: Wed, 20 Jan 2016 07:48:34 -0800 In-Reply-To: References: <1451416224-15871-1-git-send-email-jacob@teenage.engineering> <87y4cdyrbn.fsf@doppelsaurus.mobileactivedefense.com> <20151229.150843.2021692616139434395.davem@davemloft.net> <1451921108.8255.74.camel@edumazet-glaptop2.roam.corp.google.com> <1452003299.8255.87.camel@edumazet-glaptop2.roam.corp.google.com> <1452004797.8255.90.camel@edumazet-glaptop2.roam.corp.google.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, 2016-01-20 at 16:06 +0100, Jacob Siverskog wrote: > On Tue, Jan 5, 2016 at 3:39 PM, Eric Dumazet wrote: > > On Tue, 2016-01-05 at 15:34 +0100, Jacob Siverskog wrote: > >> On Tue, Jan 5, 2016 at 3:14 PM, Eric Dumazet wrote: > > > >> > > >> > You might build a kernel with KASAN support to get maybe more chances to > >> > trigger the bug. > >> > > >> > ( https://www.kernel.org/doc/Documentation/kasan.txt ) > >> > > >> > >> Ah. Doesn't seem to be supported on arm(32) unfortunately. > > > > Then you could at least use standard debugging features : > > > > CONFIG_SLAB=y > > CONFIG_SLABINFO=y > > CONFIG_DEBUG_SLAB=y > > CONFIG_DEBUG_SLAB_LEAK=y > > > > (Or equivalent SLUB options) > > > > and > > > > CONFIG_DEBUG_PAGEALLOC=y > > > > (If arm(32) has CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y) > > I tried with those enabled and while toggling power on the Bluetooth > interface I usually get this after a few iterations: > kernel: Bluetooth: Unable to push skb to HCI core(-6) Well, this code seems to be quite buggy. I do not have time to audit it, but 5 minutes are enough to spot 2 issues. skb, once given to another queue/layer should not be accessed anymore. diff --git a/drivers/bluetooth/btwilink.c b/drivers/bluetooth/btwilink.c index 24a652f9252b..2d3092aa6cfe 100644 --- a/drivers/bluetooth/btwilink.c +++ b/drivers/bluetooth/btwilink.c @@ -98,6 +98,7 @@ static void st_reg_completion_cb(void *priv_data, char data) static long st_receive(void *priv_data, struct sk_buff *skb) { struct ti_st *lhst = priv_data; + unsigned int len; int err; if (!skb) @@ -109,13 +110,14 @@ static long st_receive(void *priv_data, struct sk_buff *skb) } /* Forward skb to HCI core layer */ + len = skb->len; err = hci_recv_frame(lhst->hdev, skb); if (err < 0) { BT_ERR("Unable to push skb to HCI core(%d)", err); return err; } - lhst->hdev->stat.byte_rx += skb->len; + lhst->hdev->stat.byte_rx += len; return 0; } @@ -245,6 +247,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb) { struct ti_st *hst; long len; + u8 pkt_type; hst = hci_get_drvdata(hdev); @@ -258,6 +261,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb) * Freeing skb memory is taken care in shared transport layer, * so don't free skb memory here. */ + pkt_type = hci_skb_pkt_type(skb); len = hst->st_write(skb); if (len < 0) { kfree_skb(skb); @@ -268,7 +272,7 @@ static int ti_st_send_frame(struct hci_dev *hdev, struct sk_buff *skb) /* ST accepted our skb. So, Go ahead and do rest */ hdev->stat.byte_tx += len; - ti_st_tx_complete(hst, hci_skb_pkt_type(skb)); + ti_st_tx_complete(hst, pkt_type); return 0; }