From patchwork Mon Jan 7 20:02:23 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislav Fomichev X-Patchwork-Id: 1021574 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="H96CFyc6"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43YRBZ5phzz9sBQ for ; Tue, 8 Jan 2019 07:02:30 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726757AbfAGUC2 (ORCPT ); Mon, 7 Jan 2019 15:02:28 -0500 Received: from mail-qt1-f202.google.com ([209.85.160.202]:53291 "EHLO mail-qt1-f202.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726511AbfAGUC2 (ORCPT ); Mon, 7 Jan 2019 15:02:28 -0500 Received: by mail-qt1-f202.google.com with SMTP id d35so1315997qtd.20 for ; Mon, 07 Jan 2019 12:02:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:message-id:mime-version:subject:from:to:cc; bh=baVdTJGkepheHvmdiNMm7pJBGzsOANlfwnNNisDPOPU=; b=H96CFyc6J3NE84CM5XjuokEi7dpC7a/A/e7cs6bU/dsMIPaRC7xV7LfLhZtiSa0KsV bqpPZ0GUtSU/ABUY3KAarIlZ8lr6cKppxg5tmK7isIpbuRD966FqmwCG1JDCLSrlrH6w R3S+2gtx2U7BA9l351treSFZMZfcrYdls1ealpmh+RyM4RR6LnMC/sksG2dF/qoSN6+L Q70vfqOrn8GJ9EU2RvctqpDKbf2WqafCoab6Y+3oB85gMEX3Dwn4qjlVHnzZ2H9e/JJb smXT93x3aiCHm0T2X9ZAN812SWpOf3piYhoIQLVOotAHSx1WHWmwZb/2PEQ0k5nrfC4Z OPAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:mime-version:subject:from:to:cc; bh=baVdTJGkepheHvmdiNMm7pJBGzsOANlfwnNNisDPOPU=; b=RC9VngSUV1szwtq+uHKxLCr6G5ZWfeMMhwJ/KAX5Jx9qw6vCPSZS10ZDwrF2hPEigb 33W3jYo/tLLKN4Q5oVvLRN0NJZRRIoEEb46fDOx99WGT6tlvYxaqjv0Nt57Ntm+ocu82 hjSPW+pr/qCUyoaKJeG/dN8grLiMFP61phTVqIF+OgXoFEDrIcmVguurjmqKBGffGt2E t73y5wz+6Uxo+fGzpySgX20Cb3BAJIfiC4Vcdjrs3zuQ7hpFNkai3yHO0dg1hmYFYHLN MeVzTFUSgbCjeNmskVFig9HyBlRuRevp3HNzy5VJw4D9hJ23sKyzHpykCIC8PMetkwBB 4sQw== X-Gm-Message-State: AJcUukclbyIvqjHdWYhRrwS8hNyVvUmXR9yVkTUOiBruUb/6odHsGg09 1s2SVzsReLG9QX7s9TWP0xYtFv7NLF3TX7CSeKeEHjvms42CFYK58Kvftme22XyCfHxXpgCz1Le 7QL4pxwj+Jwb8Tjo94yYoQeEgtb3tkN8HCYL+njS4l3uQq2h0BE6XXg== X-Google-Smtp-Source: AFSGD/WGnUg7PyFuZUOLZo0eG6VTIY/FrQbL5slC7NfeAWvc7a79jGzgaFHrNxm4KttxjwXr6YdveRc= X-Received: by 2002:ac8:2c0c:: with SMTP id d12mr46747447qta.36.1546891347108; Mon, 07 Jan 2019 12:02:27 -0800 (PST) Date: Mon, 7 Jan 2019 12:02:23 -0800 Message-Id: <20190107200224.220467-1-sdf@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.20.1.97.g81188d93c3-goog Subject: [PATCH net 1/2] tun: hold napi_mutex for all napi operations From: Stanislav Fomichev To: netdev@vger.kernel.org Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com, mst@redhat.com, edumazet@google.com, Stanislav Fomichev , syzbot Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org BUG: unable to handle kernel NULL pointer dereference at 00000000000000d1 Call Trace: ? napi_gro_frags+0xa7/0x2c0 tun_get_user+0xb50/0xf20 tun_chr_write_iter+0x53/0x70 new_sync_write+0xff/0x160 vfs_write+0x191/0x1e0 __x64_sys_write+0x5e/0xd0 do_syscall_64+0x47/0xf0 entry_SYSCALL_64_after_hwframe+0x44/0xa9 I think there is a subtle race between sending a packet via tap and attaching it: CPU0: CPU1: tun_chr_ioctl(TUNSETIFF) tun_set_iff tun_attach rcu_assign_pointer(tfile->tun, tun); tun_fops->write_iter() tun_chr_write_iter tun_napi_alloc_frags napi_get_frags napi->skb = napi_alloc_skb tun_napi_init netif_napi_add napi->skb = NULL napi->skb is NULL here napi_gro_frags napi_frags_skb skb = napi->skb skb_reset_mac_header(skb) panic() To fix, do the following: * Move rcu_assign_pointer(tfile->tun, tun) to be the last thing we do in tun_attach(); this should guarantee that when we call tun_get() we always get an initialized object * As another safeguard, always grab napi_mutex whenever doing any napi operation; this should prevent napi state change between calls to napi_get_frags and napi_gro_frags Reported-by: syzbot Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver") Signed-off-by: Stanislav Fomichev --- drivers/net/tun.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index a4fdad475594..7875f06011f2 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -323,22 +323,30 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile, tfile->napi_enabled = napi_en; tfile->napi_frags_enabled = napi_en && napi_frags; if (napi_en) { + mutex_lock(&tfile->napi_mutex); netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll, NAPI_POLL_WEIGHT); napi_enable(&tfile->napi); + mutex_unlock(&tfile->napi_mutex); } } static void tun_napi_disable(struct tun_file *tfile) { - if (tfile->napi_enabled) + if (tfile->napi_enabled) { + mutex_lock(&tfile->napi_mutex); napi_disable(&tfile->napi); + mutex_unlock(&tfile->napi_mutex); + } } static void tun_napi_del(struct tun_file *tfile) { - if (tfile->napi_enabled) + if (tfile->napi_enabled) { + mutex_lock(&tfile->napi_mutex); netif_napi_del(&tfile->napi); + mutex_unlock(&tfile->napi_mutex); + } } static bool tun_napi_frags_enabled(const struct tun_file *tfile) @@ -856,7 +864,6 @@ static int tun_attach(struct tun_struct *tun, struct file *file, err = 0; } - rcu_assign_pointer(tfile->tun, tun); rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile); tun->numqueues++; @@ -876,6 +883,11 @@ static int tun_attach(struct tun_struct *tun, struct file *file, * refcnt. */ + /* All tun_fops depend on tun_get() returning non-null pointer. + * Thus, assigning tun to a tfile should be the last init operation, + * otherwise we risk using half-initialized object. + */ + rcu_assign_pointer(tfile->tun, tun); out: return err; } From patchwork Mon Jan 7 20:02:24 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stanislav Fomichev X-Patchwork-Id: 1021575 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="gYwTd7kx"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 43YRBh2HMRz9sCX for ; Tue, 8 Jan 2019 07:02:36 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726903AbfAGUCb (ORCPT ); Mon, 7 Jan 2019 15:02:31 -0500 Received: from mail-qt1-f201.google.com ([209.85.160.201]:36992 "EHLO mail-qt1-f201.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726689AbfAGUCb (ORCPT ); Mon, 7 Jan 2019 15:02:31 -0500 Received: by mail-qt1-f201.google.com with SMTP id d31so1346822qtc.4 for ; Mon, 07 Jan 2019 12:02:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=y1WQ1hbeeYGE0l/5QBpGC9iwYtdUpl1sQsoZ/jNjTi0=; b=gYwTd7kxAdrFShtCvx/gLMEGL9WtboGNThStmg5J1T8VC5r9ZssorxHgTsYgcdIBUr QVEf+i29sTa0kS1IiW5VLTBH1d0EkB7HLB1KTlXBusC+rhVLoazh68bcKmlqxo2+e8Bh 2G23iHh0n6H7llOuDFA4KmSYYSJsntCM2+UXx2muwerLPsbKRo2mtpKvyKRq9AItjBMt f6t5k3yhRiFYZJpwMhfjVGVpVoDE9tBk+IOpq/FgwhdTbeFELlzilCxAGHrLNesxbnKe Xyn41nPVqZzbOfAbNuACBXRbhQ3Q8AmUbH0A5+Q6dx3QF1rwG1k1tXwLNOWvH6C4iWlq xWVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=y1WQ1hbeeYGE0l/5QBpGC9iwYtdUpl1sQsoZ/jNjTi0=; b=KGIBG+3CC4iX/53Hkd0d/6elJerSt390tnjh5fdQW2YYgZiaEFB5cdQ5Zljfz6qCQt CV4668dG9IEHfTqU8Xs8j+aHxYho7te8w5qbQtK/XI5i6IoBde729Bds866QUgrbBnrJ /SeBi7a9zqNdkNEEqQCrvzDzn5KC8lJ6D+fOZIWSknjYEZrw86iRMlsjJwd+MuwyVURB dRQXNQeYvx1pU9msWJUOt1pPqM+eovVNuBhkjPJthNqoLkiRuyHY1hhtIdfgQttwFP8J OinNs/AZh6gUe3VTUB7pTNjyDI6hyLuCxJLcVWKI2swTGsXDMQJMp++SEb9dYJe3V2Q7 U6hw== X-Gm-Message-State: AJcUuke/qEd9MDXF3PoqbpXv+dC92wsS9c7EWqxtMH8SkYjUEWZxUtrt 95PlmD4tHw/MWhZflSqwCgqAL6XIrB1rQut7sFidaOu2SxZkDW9CVQ/SAbtz3SdNlNqMm9aAjPz g8ITFajHuE3PTq8/4NFWYRprQj28YKqG7DcGHuymPBGicn97XINmoug== X-Google-Smtp-Source: ALg8bN4htRdF6vR3oebfq7eeVoH/TwRsITnXwtRxiXnh235paDzmHrGbqvJxZXQpnkx+Bm8Scb11T20= X-Received: by 2002:a0c:af8a:: with SMTP id s10mr44288032qvc.26.1546891349600; Mon, 07 Jan 2019 12:02:29 -0800 (PST) Date: Mon, 7 Jan 2019 12:02:24 -0800 In-Reply-To: <20190107200224.220467-1-sdf@google.com> Message-Id: <20190107200224.220467-2-sdf@google.com> Mime-Version: 1.0 References: <20190107200224.220467-1-sdf@google.com> X-Mailer: git-send-email 2.20.1.97.g81188d93c3-goog Subject: [PATCH net 2/2] tun: always set skb->dev to tun->dev From: Stanislav Fomichev To: netdev@vger.kernel.org Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com, mst@redhat.com, edumazet@google.com, Stanislav Fomichev , syzbot Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org While debugging previous issue I noticed that commit 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver") started conditionally (!frags) calling eth_type_trans(skb, tun->dev) for IFF_TAP case. Since eth_type_trans sets skb->dev, some skbs can now have NULL skb->dev. Fix that by always setting skb->dev unconditionally. The syzbot fails with the following trace: WARNING: CPU: 0 PID: 11136 at net/core/flow_dissector.c:764 skb_flow_dissect_flow_keys_basic include/linux/skbuff.h:1240 [inline] skb_probe_transport_header include/linux/skbuff.h:2403 [inline] tun_get_user+0x2d4a/0x4250 drivers/net/tun.c:1906 tun_chr_write_iter+0xb9/0x160 drivers/net/tun.c:1993 call_write_iter include/linux/fs.h:1808 [inline] new_sync_write fs/read_write.c:474 [inline] But I don't think there is an actual issue since we exercise flow dissector via eth_get_headlen which doesn't use skb (and hence BPF flow dissector). But let's still properly set skb->dev so we don't have any problems going forward. Reported-by: syzbot Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver") Signed-off-by: Stanislav Fomichev --- drivers/net/tun.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 7875f06011f2..af34baf978f3 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1899,6 +1899,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, return -EINVAL; } + skb->dev = tun->dev; switch (tun->flags & TUN_TYPE_MASK) { case IFF_TUN: if (tun->flags & IFF_NO_PI) { @@ -1920,7 +1921,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile, skb_reset_mac_header(skb); skb->protocol = pi.proto; - skb->dev = tun->dev; break; case IFF_TAP: if (!frags)