All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Taehee Yoo <ap420073@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Sasha Levin <sashal@kernel.org>
Subject: [PATCH 5.5 48/65] net: rmnet: fix suspicious RCU usage
Date: Thu, 19 Mar 2020 14:04:30 +0100	[thread overview]
Message-ID: <20200319123941.528206596@linuxfoundation.org> (raw)
In-Reply-To: <20200319123926.466988514@linuxfoundation.org>

From: Taehee Yoo <ap420073@gmail.com>

[ Upstream commit 102210f7664442d8c0ce332c006ea90626df745b ]

rmnet_get_port() internally calls rcu_dereference_rtnl(),
which checks RTNL.
But rmnet_get_port() could be called by packet path.
The packet path is not protected by RTNL.
So, the suspicious RCU usage problem occurs.

Test commands:
    modprobe rmnet
    ip netns add nst
    ip link add veth0 type veth peer name veth1
    ip link set veth1 netns nst
    ip link add rmnet0 link veth0 type rmnet mux_id 1
    ip netns exec nst ip link add rmnet1 link veth1 type rmnet mux_id 1
    ip netns exec nst ip link set veth1 up
    ip netns exec nst ip link set rmnet1 up
    ip netns exec nst ip a a 192.168.100.2/24 dev rmnet1
    ip link set veth0 up
    ip link set rmnet0 up
    ip a a 192.168.100.1/24 dev rmnet0
    ping 192.168.100.2

Splat looks like:
[  146.630958][ T1174] WARNING: suspicious RCU usage
[  146.631735][ T1174] 5.6.0-rc1+ #447 Not tainted
[  146.632387][ T1174] -----------------------------
[  146.633151][ T1174] drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c:386 suspicious rcu_dereference_check() !
[  146.634742][ T1174]
[  146.634742][ T1174] other info that might help us debug this:
[  146.634742][ T1174]
[  146.645992][ T1174]
[  146.645992][ T1174] rcu_scheduler_active = 2, debug_locks = 1
[  146.646937][ T1174] 5 locks held by ping/1174:
[  146.647609][ T1174]  #0: ffff8880c31dea70 (sk_lock-AF_INET){+.+.}, at: raw_sendmsg+0xab8/0x2980
[  146.662463][ T1174]  #1: ffffffff93925660 (rcu_read_lock_bh){....}, at: ip_finish_output2+0x243/0x2150
[  146.671696][ T1174]  #2: ffffffff93925660 (rcu_read_lock_bh){....}, at: __dev_queue_xmit+0x213/0x2940
[  146.673064][ T1174]  #3: ffff8880c19ecd58 (&dev->qdisc_running_key#7){+...}, at: ip_finish_output2+0x714/0x2150
[  146.690358][ T1174]  #4: ffff8880c5796898 (&dev->qdisc_xmit_lock_key#3){+.-.}, at: sch_direct_xmit+0x1e2/0x1020
[  146.699875][ T1174]
[  146.699875][ T1174] stack backtrace:
[  146.701091][ T1174] CPU: 0 PID: 1174 Comm: ping Not tainted 5.6.0-rc1+ #447
[  146.705215][ T1174] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  146.706565][ T1174] Call Trace:
[  146.707102][ T1174]  dump_stack+0x96/0xdb
[  146.708007][ T1174]  rmnet_get_port.part.9+0x76/0x80 [rmnet]
[  146.709233][ T1174]  rmnet_egress_handler+0x107/0x420 [rmnet]
[  146.710492][ T1174]  ? sch_direct_xmit+0x1e2/0x1020
[  146.716193][ T1174]  rmnet_vnd_start_xmit+0x3d/0xa0 [rmnet]
[  146.717012][ T1174]  dev_hard_start_xmit+0x160/0x740
[  146.717854][ T1174]  sch_direct_xmit+0x265/0x1020
[  146.718577][ T1174]  ? register_lock_class+0x14d0/0x14d0
[  146.719429][ T1174]  ? dev_watchdog+0xac0/0xac0
[  146.723738][ T1174]  ? __dev_queue_xmit+0x15fd/0x2940
[  146.724469][ T1174]  ? lock_acquire+0x164/0x3b0
[  146.725172][ T1174]  __dev_queue_xmit+0x20c7/0x2940
[ ... ]

Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial implementation")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c  | 13 ++++++-------
 drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h  |  2 +-
 .../net/ethernet/qualcomm/rmnet/rmnet_handlers.c    |  4 ++--
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
index ac58f584190bd..fc68ecdd804bc 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.c
@@ -382,11 +382,10 @@ struct rtnl_link_ops rmnet_link_ops __read_mostly = {
 	.fill_info	= rmnet_fill_info,
 };
 
-/* Needs either rcu_read_lock() or rtnl lock */
-struct rmnet_port *rmnet_get_port(struct net_device *real_dev)
+struct rmnet_port *rmnet_get_port_rcu(struct net_device *real_dev)
 {
 	if (rmnet_is_real_dev_registered(real_dev))
-		return rcu_dereference_rtnl(real_dev->rx_handler_data);
+		return rcu_dereference_bh(real_dev->rx_handler_data);
 	else
 		return NULL;
 }
@@ -412,7 +411,7 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
 	struct rmnet_port *port, *slave_port;
 	int err;
 
-	port = rmnet_get_port(real_dev);
+	port = rmnet_get_port_rtnl(real_dev);
 
 	/* If there is more than one rmnet dev attached, its probably being
 	 * used for muxing. Skip the briding in that case
@@ -427,7 +426,7 @@ int rmnet_add_bridge(struct net_device *rmnet_dev,
 	if (err)
 		return -EBUSY;
 
-	slave_port = rmnet_get_port(slave_dev);
+	slave_port = rmnet_get_port_rtnl(slave_dev);
 	slave_port->rmnet_mode = RMNET_EPMODE_BRIDGE;
 	slave_port->bridge_ep = real_dev;
 
@@ -445,11 +444,11 @@ int rmnet_del_bridge(struct net_device *rmnet_dev,
 	struct net_device *real_dev = priv->real_dev;
 	struct rmnet_port *port, *slave_port;
 
-	port = rmnet_get_port(real_dev);
+	port = rmnet_get_port_rtnl(real_dev);
 	port->rmnet_mode = RMNET_EPMODE_VND;
 	port->bridge_ep = NULL;
 
-	slave_port = rmnet_get_port(slave_dev);
+	slave_port = rmnet_get_port_rtnl(slave_dev);
 	rmnet_unregister_real_device(slave_dev, slave_port);
 
 	netdev_dbg(slave_dev, "removed from rmnet as slave\n");
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
index cd0a6bcbe74ad..0d568dcfd65a1 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_config.h
@@ -65,7 +65,7 @@ struct rmnet_priv {
 	struct rmnet_priv_stats stats;
 };
 
-struct rmnet_port *rmnet_get_port(struct net_device *real_dev);
+struct rmnet_port *rmnet_get_port_rcu(struct net_device *real_dev);
 struct rmnet_endpoint *rmnet_get_endpoint(struct rmnet_port *port, u8 mux_id);
 int rmnet_add_bridge(struct net_device *rmnet_dev,
 		     struct net_device *slave_dev,
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 1b74bc1604027..074a8b326c304 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -184,7 +184,7 @@ rx_handler_result_t rmnet_rx_handler(struct sk_buff **pskb)
 		return RX_HANDLER_PASS;
 
 	dev = skb->dev;
-	port = rmnet_get_port(dev);
+	port = rmnet_get_port_rcu(dev);
 
 	switch (port->rmnet_mode) {
 	case RMNET_EPMODE_VND:
@@ -217,7 +217,7 @@ void rmnet_egress_handler(struct sk_buff *skb)
 	skb->dev = priv->real_dev;
 	mux_id = priv->mux_id;
 
-	port = rmnet_get_port(skb->dev);
+	port = rmnet_get_port_rcu(skb->dev);
 	if (!port)
 		goto drop;
 
-- 
2.20.1




  parent reply	other threads:[~2020-03-19 13:26 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-19 13:03 [PATCH 5.5 00/65] 5.5.11-rc1 review Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 01/65] gpiolib: Add support for the irqdomain which doesnt use irq_fwspec as arg Greg Kroah-Hartman
2020-03-19 13:33   ` Kevin Hao
2020-03-19 13:47     ` Greg Kroah-Hartman
2020-03-19 14:47       ` Kevin Hao
2020-03-19 14:58         ` Greg Kroah-Hartman
2020-03-19 22:27         ` Linus Walleij
2020-03-19 13:03 ` [PATCH 5.5 02/65] pinctrl: qcom: ssbi-gpio: Fix fwspec parsing bug Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 03/65] mmc: core: Default to generic_cmd6_time as timeout in __mmc_switch() Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 04/65] mmc: core: Allow host controllers to require R1B for CMD6 Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 05/65] mmc: sdhci-tegra: Fix busy detection by enabling MMC_CAP_NEED_RSP_BUSY Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 06/65] mmc: sdhci-omap: " Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 07/65] mmc: core: Respect MMC_CAP_NEED_RSP_BUSY for eMMC sleep command Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 08/65] mmc: core: Respect MMC_CAP_NEED_RSP_BUSY for erase/trim/discard Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 09/65] ACPI: watchdog: Allow disabling WDAT at boot Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 10/65] HID: apple: Add support for recent firmware on Magic Keyboards Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 11/65] ACPI: watchdog: Set default timeout in probe Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 12/65] HID: i2c-hid: add Trekstor Surfbook E11B to descriptor override Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 13/65] mips: vdso: fix jalr t9 crash in vdso code Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 14/65] MIPS: Disable VDSO time functionality on microMIPS Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 15/65] mips: vdso: add build time check that no jalr t9 calls left Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 16/65] HID: hid-bigbenff: fix general protection fault caused by double kfree Greg Kroah-Hartman
2020-03-19 13:03 ` [PATCH 5.5 17/65] HID: hid-bigbenff: call hid_hw_stop() in case of error Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 18/65] HID: hid-bigbenff: fix race condition for scheduled work during removal Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 19/65] riscv: set pmp configuration if kernel is running in M-mode Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 20/65] MIPS: vdso: Wrap -mexplicit-relocs in cc-option Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 21/65] kunit: run kunit_tool from any directory Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 22/65] selftests/rseq: Fix out-of-tree compilation Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 23/65] tracing: Fix number printing bug in print_synth_event() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 24/65] cfg80211: check reg_rule for NULL in handle_channel_custom() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 25/65] scsi: libfc: free response frame from GPN_ID Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 26/65] net: usb: qmi_wwan: restore mtu min/max values after raw_ip switch Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 27/65] net: ks8851-ml: Fix IRQ handling and locking Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 28/65] mac80211: rx: avoid RCU list traversal under mutex Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 29/65] net: ll_temac: Fix race condition causing TX hang Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 30/65] net: ll_temac: Add more error handling of dma_map_single() calls Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 31/65] net: ll_temac: Fix RX buffer descriptor handling on GFP_ATOMIC pressure Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 32/65] net: ll_temac: Handle DMA halt condition caused by buffer underrun Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 33/65] blk-mq: insert passthrough request into hctx->dispatch directly Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 34/65] io_uring: fix poll_list race for SETUP_IOPOLL|SETUP_SQPOLL Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 35/65] drm/amdgpu: fix memory leak during TDR test(v2) Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 36/65] io_uring: pick up link work on submit reference drop Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 37/65] kbuild: add dtbs_check to PHONY Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 38/65] kbuild: add dt_binding_check to PHONY in a correct place Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 39/65] signal: avoid double atomic counter increments for user accounting Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 40/65] net: bcmgenet: Clear ID_MODE_DIS in EXT_RGMII_OOB_CTRL when not needed Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 41/65] slip: not call free_netdev before rtnl_unlock in slip_open Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 42/65] net: phy: mscc: fix firmware paths Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 43/65] hinic: fix a irq affinity bug Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 44/65] hinic: fix a bug of setting hw_ioctxt Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 45/65] hinic: fix a bug of rss configuration Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 46/65] net: rmnet: fix NULL pointer dereference in rmnet_newlink() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 47/65] net: rmnet: fix NULL pointer dereference in rmnet_changelink() Greg Kroah-Hartman
2020-03-19 13:04 ` Greg Kroah-Hartman [this message]
2020-03-19 13:04 ` [PATCH 5.5 49/65] net: rmnet: remove rcu_read_lock in rmnet_force_unassociate_device() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 50/65] net: rmnet: do not allow to change mux id if mux id is duplicated Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 51/65] net: rmnet: use upper/lower device infrastructure Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 52/65] net: rmnet: fix bridge mode bugs Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 53/65] net: rmnet: fix packet forwarding in rmnet bridge mode Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 54/65] sfc: fix timestamp reconstruction at 16-bit rollover points Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 55/65] mlxsw: pci: Wait longer before accessing the device after reset Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 56/65] net: dsa: mv88e6xxx: Fix masking of egress port Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 57/65] jbd2: fix data races at struct journal_head Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 58/65] blk-mq: insert flush request to the front of dispatch queue Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 59/65] ARM: 8957/1: VDSO: Match ARMv8 timer in cntvct_functional() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 60/65] ARM: 8958/1: rename missed uaccess .fixup section Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 61/65] mm: slub: add missing TID bump in kmem_cache_alloc_bulk() Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 62/65] HID: google: add moonball USB id Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 63/65] HID: add ALWAYS_POLL quirk to lenovo pixart mouse Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 64/65] ARM: 8961/2: Fix Kbuild issue caused by per-task stack protector GCC plugin Greg Kroah-Hartman
2020-03-19 13:04 ` [PATCH 5.5 65/65] ipv4: ensure rcu_read_lock() in cipso_v4_error() Greg Kroah-Hartman
2020-03-19 14:44 ` [PATCH 5.5 00/65] 5.5.11-rc1 review Guenter Roeck
2020-03-19 14:59   ` Greg Kroah-Hartman
2020-03-19 15:15     ` Guenter Roeck
2020-03-19 15:33       ` Greg Kroah-Hartman
2020-03-20 14:46       ` Sasha Levin
2020-03-22 19:51         ` Pavel Machek
2020-03-22 20:34           ` Sasha Levin
2020-03-23  6:48           ` Greg Kroah-Hartman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200319123941.528206596@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=ap420073@gmail.com \
    --cc=davem@davemloft.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sashal@kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.