Netdev List
 help / color / mirror / Atom feed
* [PATCH net 03/11] ehea: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ehea uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Douglas Miller <dougmill@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ehea/ehea_main.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ehea/ehea_main.c b/drivers/net/ethernet/ibm/ehea/ehea_main.c
index ba580bfae512326d346e96e67718bec10ed716cc..03f64f40b2a3e0a3d9f3432cb6fbdd7bcd264a4c 100644
--- a/drivers/net/ethernet/ibm/ehea/ehea_main.c
+++ b/drivers/net/ethernet/ibm/ehea/ehea_main.c
@@ -921,17 +921,6 @@ static int ehea_poll(struct napi_struct *napi, int budget)
 	return rx;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ehea_netpoll(struct net_device *dev)
-{
-	struct ehea_port *port = netdev_priv(dev);
-	int i;
-
-	for (i = 0; i < port->num_def_qps; i++)
-		napi_schedule(&port->port_res[i].napi);
-}
-#endif
-
 static irqreturn_t ehea_recv_irq_handler(int irq, void *param)
 {
 	struct ehea_port_res *pr = param;
@@ -2953,9 +2942,6 @@ static const struct net_device_ops ehea_netdev_ops = {
 	.ndo_open		= ehea_open,
 	.ndo_stop		= ehea_stop,
 	.ndo_start_xmit		= ehea_start_xmit,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ehea_netpoll,
-#endif
 	.ndo_get_stats64	= ehea_get_stats64,
 	.ndo_set_mac_address	= ehea_set_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 04/11] net: hns: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Salil Mehta
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

hns uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Yisen Zhuang <yisen.zhuang@huawei.com>
Cc: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index 5ce23d4b717ec2004d9fea5a222c7f0ba48eee06..28e907831b0eddbf760e0edb579ae7ae708520e0 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -1503,21 +1503,6 @@ static int hns_nic_do_ioctl(struct net_device *netdev, struct ifreq *ifr,
 	return phy_mii_ioctl(phy_dev, ifr, cmd);
 }
 
-/* use only for netconsole to poll with the device without interrupt */
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void hns_nic_poll_controller(struct net_device *ndev)
-{
-	struct hns_nic_priv *priv = netdev_priv(ndev);
-	unsigned long flags;
-	int i;
-
-	local_irq_save(flags);
-	for (i = 0; i < priv->ae_handle->q_num * 2; i++)
-		napi_schedule(&priv->ring_data[i].napi);
-	local_irq_restore(flags);
-}
-#endif
-
 static netdev_tx_t hns_nic_net_xmit(struct sk_buff *skb,
 				    struct net_device *ndev)
 {
@@ -1970,9 +1955,6 @@ static const struct net_device_ops hns_nic_netdev_ops = {
 	.ndo_set_features = hns_nic_set_features,
 	.ndo_fix_features = hns_nic_fix_features,
 	.ndo_get_stats64 = hns_nic_get_stats64,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = hns_nic_poll_controller,
-#endif
 	.ndo_set_rx_mode = hns_nic_set_rx_mode,
 	.ndo_select_queue = hns_nic_select_queue,
 };
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 05/11] virtio_net: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

virto_net uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
 drivers/net/virtio_net.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 76592090522607a4bdf5422b1d49ec99c6fd68ac..dab504ec5e502be401cbfe9a8e3f0f572c0220ba 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1699,17 +1699,6 @@ static void virtnet_stats(struct net_device *dev,
 	tot->rx_frame_errors = dev->stats.rx_frame_errors;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void virtnet_netpoll(struct net_device *dev)
-{
-	struct virtnet_info *vi = netdev_priv(dev);
-	int i;
-
-	for (i = 0; i < vi->curr_queue_pairs; i++)
-		napi_schedule(&vi->rq[i].napi);
-}
-#endif
-
 static void virtnet_ack_link_announce(struct virtnet_info *vi)
 {
 	rtnl_lock();
@@ -2447,9 +2436,6 @@ static const struct net_device_ops virtnet_netdev = {
 	.ndo_get_stats64     = virtnet_stats,
 	.ndo_vlan_rx_add_vid = virtnet_vlan_rx_add_vid,
 	.ndo_vlan_rx_kill_vid = virtnet_vlan_rx_kill_vid,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = virtnet_netpoll,
-#endif
 	.ndo_bpf		= virtnet_xdp,
 	.ndo_xdp_xmit		= virtnet_xdp_xmit,
 	.ndo_features_check	= passthru_features_check,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 06/11] qlcnic: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

qlcnic uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Harish Patil <harish.patil@cavium.com>
Cc: Manish Chopra <manish.chopra@cavium.com>
---
 .../net/ethernet/qlogic/qlcnic/qlcnic_main.c  | 45 -------------------
 1 file changed, 45 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
index 2d38d1ac2aae58fd210030c7b143011f76b921cc..dbd48012224f2467d27134eedc692a68b92b1a04 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c
@@ -59,9 +59,6 @@ static int qlcnic_close(struct net_device *netdev);
 static void qlcnic_tx_timeout(struct net_device *netdev);
 static void qlcnic_attach_work(struct work_struct *work);
 static void qlcnic_fwinit_work(struct work_struct *work);
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void qlcnic_poll_controller(struct net_device *netdev);
-#endif
 
 static void qlcnic_idc_debug_info(struct qlcnic_adapter *adapter, u8 encoding);
 static int qlcnic_can_start_firmware(struct qlcnic_adapter *adapter);
@@ -545,9 +542,6 @@ static const struct net_device_ops qlcnic_netdev_ops = {
 	.ndo_udp_tunnel_add	= qlcnic_add_vxlan_port,
 	.ndo_udp_tunnel_del	= qlcnic_del_vxlan_port,
 	.ndo_features_check	= qlcnic_features_check,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = qlcnic_poll_controller,
-#endif
 #ifdef CONFIG_QLCNIC_SRIOV
 	.ndo_set_vf_mac		= qlcnic_sriov_set_vf_mac,
 	.ndo_set_vf_rate	= qlcnic_sriov_set_vf_tx_rate,
@@ -3200,45 +3194,6 @@ static irqreturn_t qlcnic_msix_tx_intr(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void qlcnic_poll_controller(struct net_device *netdev)
-{
-	struct qlcnic_adapter *adapter = netdev_priv(netdev);
-	struct qlcnic_host_sds_ring *sds_ring;
-	struct qlcnic_recv_context *recv_ctx;
-	struct qlcnic_host_tx_ring *tx_ring;
-	int ring;
-
-	if (!test_bit(__QLCNIC_DEV_UP, &adapter->state))
-		return;
-
-	recv_ctx = adapter->recv_ctx;
-
-	for (ring = 0; ring < adapter->drv_sds_rings; ring++) {
-		sds_ring = &recv_ctx->sds_rings[ring];
-		qlcnic_disable_sds_intr(adapter, sds_ring);
-		napi_schedule(&sds_ring->napi);
-	}
-
-	if (adapter->flags & QLCNIC_MSIX_ENABLED) {
-		/* Only Multi-Tx queue capable devices need to
-		 * schedule NAPI for TX rings
-		 */
-		if ((qlcnic_83xx_check(adapter) &&
-		     (adapter->flags & QLCNIC_TX_INTR_SHARED)) ||
-		    (qlcnic_82xx_check(adapter) &&
-		     !qlcnic_check_multi_tx(adapter)))
-			return;
-
-		for (ring = 0; ring < adapter->drv_tx_rings; ring++) {
-			tx_ring = &adapter->tx_ring[ring];
-			qlcnic_disable_tx_intr(adapter, tx_ring);
-			napi_schedule(&tx_ring->napi);
-		}
-	}
-}
-#endif
-
 static void
 qlcnic_idc_debug_info(struct qlcnic_adapter *adapter, u8 encoding)
 {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 07/11] qlogic: netxen: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Rahul Verma
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

netxen uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Manish Chopra <manish.chopra@cavium.com>
Cc: Rahul Verma <rahul.verma@cavium.com>
---
 .../ethernet/qlogic/netxen/netxen_nic_main.c  | 23 -------------------
 1 file changed, 23 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
index 69aa7fc392c5e4ad1cbcd9025f56bffdf3aa92c7..59c70be22a84c11262388529cf0ddf09887cea96 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c
@@ -72,9 +72,6 @@ static void netxen_schedule_work(struct netxen_adapter *adapter,
 		work_func_t func, int delay);
 static void netxen_cancel_fw_work(struct netxen_adapter *adapter);
 static int netxen_nic_poll(struct napi_struct *napi, int budget);
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netxen_nic_poll_controller(struct net_device *netdev);
-#endif
 
 static void netxen_create_sysfs_entries(struct netxen_adapter *adapter);
 static void netxen_remove_sysfs_entries(struct netxen_adapter *adapter);
@@ -581,9 +578,6 @@ static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_tx_timeout	   = netxen_tx_timeout,
 	.ndo_fix_features = netxen_fix_features,
 	.ndo_set_features = netxen_set_features,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = netxen_nic_poll_controller,
-#endif
 };
 
 static inline bool netxen_function_zero(struct pci_dev *pdev)
@@ -2402,23 +2396,6 @@ static int netxen_nic_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void netxen_nic_poll_controller(struct net_device *netdev)
-{
-	int ring;
-	struct nx_host_sds_ring *sds_ring;
-	struct netxen_adapter *adapter = netdev_priv(netdev);
-	struct netxen_recv_context *recv_ctx = &adapter->recv_ctx;
-
-	disable_irq(adapter->irq);
-	for (ring = 0; ring < adapter->max_sds_rings; ring++) {
-		sds_ring = &recv_ctx->sds_rings[ring];
-		netxen_intr(adapter->irq, sds_ring);
-	}
-	enable_irq(adapter->irq);
-}
-#endif
-
 static int
 nx_incr_dev_ref_cnt(struct netxen_adapter *adapter)
 {
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 08/11] net: ena: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Saeed Bishara,
	Zorik Machulsky
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ena uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Netanel Belgazal <netanel@amazon.com>
Cc: Saeed Bishara <saeedb@amazon.com>
Cc: Zorik Machulsky <zorik@amazon.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 22 --------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 29b5774dd32d47e5ad2c43e78b599493f011d129..25621a218f20754c29963c7ded3075c0c89a232c 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -2185,25 +2185,6 @@ static netdev_tx_t ena_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	return NETDEV_TX_OK;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ena_netpoll(struct net_device *netdev)
-{
-	struct ena_adapter *adapter = netdev_priv(netdev);
-	int i;
-
-	/* Dont schedule NAPI if the driver is in the middle of reset
-	 * or netdev is down.
-	 */
-
-	if (!test_bit(ENA_FLAG_DEV_UP, &adapter->flags) ||
-	    test_bit(ENA_FLAG_TRIGGER_RESET, &adapter->flags))
-		return;
-
-	for (i = 0; i < adapter->num_queues; i++)
-		napi_schedule(&adapter->ena_napi[i].napi);
-}
-#endif /* CONFIG_NET_POLL_CONTROLLER */
-
 static u16 ena_select_queue(struct net_device *dev, struct sk_buff *skb,
 			    struct net_device *sb_dev,
 			    select_queue_fallback_t fallback)
@@ -2369,9 +2350,6 @@ static const struct net_device_ops ena_netdev_ops = {
 	.ndo_change_mtu		= ena_change_mtu,
 	.ndo_set_mac_address	= NULL,
 	.ndo_validate_addr	= eth_validate_addr,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ena_netpoll,
-#endif /* CONFIG_NET_POLL_CONTROLLER */
 };
 
 static int ena_device_validate_params(struct ena_adapter *adapter,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 09/11] sfc: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:31 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Edward Cree,
	Bert Kenward
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

sfc uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Bert Kenward <bkenward@solarflare.com>
Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
---
 drivers/net/ethernet/sfc/efx.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 330233286e785254f5f29c87f9557a305974f606..3d0dd39c289e05b8a7a6778363461ef5698dc62b 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -2206,29 +2206,6 @@ static void efx_fini_napi(struct efx_nic *efx)
 		efx_fini_napi_channel(channel);
 }
 
-/**************************************************************************
- *
- * Kernel netpoll interface
- *
- *************************************************************************/
-
-#ifdef CONFIG_NET_POLL_CONTROLLER
-
-/* Although in the common case interrupts will be disabled, this is not
- * guaranteed. However, all our work happens inside the NAPI callback,
- * so no locking is required.
- */
-static void efx_netpoll(struct net_device *net_dev)
-{
-	struct efx_nic *efx = netdev_priv(net_dev);
-	struct efx_channel *channel;
-
-	efx_for_each_channel(channel, efx)
-		efx_schedule_channel(channel);
-}
-
-#endif
-
 /**************************************************************************
  *
  * Kernel net device interface
@@ -2509,9 +2486,6 @@ static const struct net_device_ops efx_netdev_ops = {
 #endif
 	.ndo_get_phys_port_id   = efx_get_phys_port_id,
 	.ndo_get_phys_port_name	= efx_get_phys_port_name,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = efx_netpoll,
-#endif
 	.ndo_setup_tc		= efx_setup_tc,
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= efx_filter_rfs,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 10/11] sfc-falcon: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:32 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, Edward Cree,
	Bert Kenward
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

sfc-falcon uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Solarflare linux maintainers <linux-net-drivers@solarflare.com>
Cc: Edward Cree <ecree@solarflare.com>
Cc: Bert Kenward <bkenward@solarflare.com>
---
 drivers/net/ethernet/sfc/falcon/efx.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/drivers/net/ethernet/sfc/falcon/efx.c b/drivers/net/ethernet/sfc/falcon/efx.c
index dd5530a4f8c8936868aed7171bd9481f93730d76..03e2455c502eacd9a4fd5c7fd320a9edcf265f77 100644
--- a/drivers/net/ethernet/sfc/falcon/efx.c
+++ b/drivers/net/ethernet/sfc/falcon/efx.c
@@ -2052,29 +2052,6 @@ static void ef4_fini_napi(struct ef4_nic *efx)
 		ef4_fini_napi_channel(channel);
 }
 
-/**************************************************************************
- *
- * Kernel netpoll interface
- *
- *************************************************************************/
-
-#ifdef CONFIG_NET_POLL_CONTROLLER
-
-/* Although in the common case interrupts will be disabled, this is not
- * guaranteed. However, all our work happens inside the NAPI callback,
- * so no locking is required.
- */
-static void ef4_netpoll(struct net_device *net_dev)
-{
-	struct ef4_nic *efx = netdev_priv(net_dev);
-	struct ef4_channel *channel;
-
-	ef4_for_each_channel(channel, efx)
-		ef4_schedule_channel(channel);
-}
-
-#endif
-
 /**************************************************************************
  *
  * Kernel net device interface
@@ -2250,9 +2227,6 @@ static const struct net_device_ops ef4_netdev_ops = {
 	.ndo_set_mac_address	= ef4_set_mac_address,
 	.ndo_set_rx_mode	= ef4_set_rx_mode,
 	.ndo_set_features	= ef4_set_features,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller = ef4_netpoll,
-#endif
 	.ndo_setup_tc		= ef4_setup_tc,
 #ifdef CONFIG_RFS_ACCEL
 	.ndo_rx_flow_steer	= ef4_filter_rfs,
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH net 11/11] ibmvnic: remove ndo_poll_controller
From: Eric Dumazet @ 2018-09-27 16:32 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Michael Chan, Aviad Krawczyk, Song Liu,
	Douglas Miller, Yisen Zhuang, Michael S . Tsirkin, Jason Wang,
	Harish Patil, Manish Chopra, Netanel Belgazal,
	Solarflare linux maintainers, Thomas Falcon, John Allen
In-Reply-To: <20180927163201.56609-1-edumazet@google.com>

As diagnosed by Song Liu, ndo_poll_controller() can
be very dangerous on loaded hosts, since the cpu
calling ndo_poll_controller() might steal all NAPI
contexts (for all RX/TX queues of the NIC). This capture
can last for unlimited amount of time, since one
cpu is generally not able to drain all the queues under load.

ibmvnic uses NAPI for TX completions, so we better let core
networking stack call the napi->poll() to avoid the capture.

ibmvnic_netpoll_controller() was completely wrong anyway,
as it was scheduling NAPI to service RX queues (instead of TX),
so I doubt netpoll ever worked on this driver.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Cc: John Allen <jallen@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 4f0daf67b18df2dcf11d7a406ebc1982e0fee466..699ef942b615c3a22053ba1419399317a6642cfa 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2207,19 +2207,6 @@ static int ibmvnic_poll(struct napi_struct *napi, int budget)
 	return frames_processed;
 }
 
-#ifdef CONFIG_NET_POLL_CONTROLLER
-static void ibmvnic_netpoll_controller(struct net_device *dev)
-{
-	struct ibmvnic_adapter *adapter = netdev_priv(dev);
-	int i;
-
-	replenish_pools(netdev_priv(dev));
-	for (i = 0; i < adapter->req_rx_queues; i++)
-		ibmvnic_interrupt_rx(adapter->rx_scrq[i]->irq,
-				     adapter->rx_scrq[i]);
-}
-#endif
-
 static int wait_for_reset(struct ibmvnic_adapter *adapter)
 {
 	int rc, ret;
@@ -2292,9 +2279,6 @@ static const struct net_device_ops ibmvnic_netdev_ops = {
 	.ndo_set_mac_address	= ibmvnic_set_mac,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= ibmvnic_tx_timeout,
-#ifdef CONFIG_NET_POLL_CONTROLLER
-	.ndo_poll_controller	= ibmvnic_netpoll_controller,
-#endif
 	.ndo_change_mtu		= ibmvnic_change_mtu,
 	.ndo_features_check     = ibmvnic_features_check,
 };
-- 
2.19.0.605.g01d371f741-goog

^ permalink raw reply related

* [PATCH v2 bpf-next] bpf: test_bpf: add init_net to dev for flow_dissector
From: Song Liu @ 2018-09-27 16:34 UTC (permalink / raw)
  To: netdev; +Cc: daniel, kernel-team, Song Liu, Willem de Bruijn, Petar Penkov

Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
However, this is not true for test_bpf. As a result, test_bpf.ko crashes
the system with the following stack trace:

[ 1133.716622] BUG: unable to handle kernel paging request at 0000000000001030
[ 1133.716623] PGD 8000001fbf7ee067
[ 1133.716624] P4D 8000001fbf7ee067
[ 1133.716624] PUD 1f6c1cf067
[ 1133.716625] PMD 0
[ 1133.716628] Oops: 0000 [#1] SMP PTI
[ 1133.716630] CPU: 7 PID: 40473 Comm: modprobe Kdump: loaded Not tainted 4.19.0-rc5-00805-gca11cc92ccd2 #1167
[ 1133.716631] Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM12.5 12/06/2017
[ 1133.716638] RIP: 0010:__skb_flow_dissect+0x83/0x1680
[ 1133.716639] Code: 04 00 00 41 0f b7 44 24 04 48 85 db 4d 8d 14 07 0f 84 01 02 00 00 48 8b 43 10 48 85 c0 0f 84 e5 01 00 00 48 8b 80 a8 04 00 00 <48> 8b 90 30 10 00 00 48 85 d2 0f 84 dd 01 00 00 31 c0 b9 05 00 00
[ 1133.716640] RSP: 0018:ffffc900303c7a80 EFLAGS: 00010282
[ 1133.716642] RAX: 0000000000000000 RBX: ffff881fea0b7400 RCX: 0000000000000000
[ 1133.716643] RDX: ffffc900303c7bb4 RSI: ffffffff8235c3e0 RDI: ffff881fea0b7400
[ 1133.716643] RBP: ffffc900303c7b80 R08: 0000000000000000 R09: 000000000000000e
[ 1133.716644] R10: ffffc900303c7bb4 R11: ffff881fb6840400 R12: ffffffff8235c3e0
[ 1133.716645] R13: 0000000000000008 R14: 000000000000001e R15: ffffc900303c7bb4
[ 1133.716646] FS:  00007f54e75d3740(0000) GS:ffff881fff5c0000(0000) knlGS:0000000000000000
[ 1133.716648] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1133.716649] CR2: 0000000000001030 CR3: 0000001f6c226005 CR4: 00000000003606e0
[ 1133.716649] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1133.716650] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1133.716651] Call Trace:
[ 1133.716660]  ? sched_clock_cpu+0xc/0xa0
[ 1133.716662]  ? sched_clock_cpu+0xc/0xa0
[ 1133.716665]  ? log_store+0x1b5/0x260
[ 1133.716667]  ? up+0x12/0x60
[ 1133.716669]  ? skb_get_poff+0x4b/0xa0
[ 1133.716674]  ? __kmalloc_reserve.isra.47+0x2e/0x80
[ 1133.716675]  skb_get_poff+0x4b/0xa0
[ 1133.716680]  bpf_skb_get_pay_offset+0xa/0x10
[ 1133.716686]  ? test_bpf_init+0x578/0x1000 [test_bpf]
[ 1133.716690]  ? netlink_broadcast_filtered+0x153/0x3d0
[ 1133.716695]  ? free_pcppages_bulk+0x324/0x600
[ 1133.716696]  ? 0xffffffffa0279000
[ 1133.716699]  ? do_one_initcall+0x46/0x1bd
[ 1133.716704]  ? kmem_cache_alloc_trace+0x144/0x1a0
[ 1133.716709]  ? do_init_module+0x5b/0x209
[ 1133.716712]  ? load_module+0x2136/0x25d0
[ 1133.716715]  ? __do_sys_finit_module+0xba/0xe0
[ 1133.716717]  ? __do_sys_finit_module+0xba/0xe0
[ 1133.716719]  ? do_syscall_64+0x48/0x100
[ 1133.716724]  ? entry_SYSCALL_64_after_hwframe+0x44/0xa9

This patch fixes tes_bpf by using init_net in the dummy dev.

Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
Reported-by: Eric Dumazet <edumazet@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Petar Penkov <ppenkov@google.com>
Signed-off-by: Song Liu <songliubraving@fb.com>
---
 lib/test_bpf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 08d3d59dca17..aa22bcaec1dc 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6494,6 +6494,7 @@ static struct sk_buff *populate_skb(char *buf, int size)
 	skb->queue_mapping = SKB_QUEUE_MAP;
 	skb->vlan_tci = SKB_VLAN_TCI;
 	skb->vlan_proto = htons(ETH_P_IP);
+	dev_net_set(&dev, &init_net);
 	skb->dev = &dev;
 	skb->dev->ifindex = SKB_DEV_IFINDEX;
 	skb->dev->type = SKB_DEV_TYPE;
-- 
2.17.1

^ permalink raw reply related

* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Andrew Lunn @ 2018-09-27 16:38 UTC (permalink / raw)
  To: Chris Preimesberger
  Cc: Eran Ben Elisha, Neil Horman, linville@tuxdriver.com,
	netdev@vger.kernel.org
In-Reply-To: <82CEAF9FFBA4DD428B132074FB91DF7D5F6489E3@CSI-MAILSRV.csicompanies.internal>

On Thu, Sep 27, 2018 at 04:08:24PM +0000, Chris Preimesberger wrote:
> Please correct me if I'm wrong, but...
> It looks like Eran's proposed fix would remove all warning and
> alarm indications from ethtool's output. It's worth mentioning
> that for me, the following fields always reported correctly
> as Off while no alarm condition was present
> and On while alarm condition(s) were present
> *per the QSFP's true/programmed threshold values*
> *not per the incorrectly reported threshold values*

These alarm values are in the first page. So the information the
driver returns does contain this information. What is missing is the
thresholds, which are not provided by the driver.

But there is a comment in the code:

        /*
         * There is no clear identifier to signify the existence of
         * optical diagnostics similar to SFF-8472. So checking existence
         * of page 3, will provide the gurantee for existence of alarms
         * and thresholds
         * If pagging support exists, then supports_alarms is marked as 1
         */

These alarm values are optional. The spec says so. So in order to
decide if they are implemented, ethtool looks to see if the thresholds
are available. If there are thresholds, it makes sense the alarms are
implemented.

Unfortunately, the driver never returns the thresholds. So ethtool has
no real choice and won't display the alarms since it cannot determine
if they are valid.

In order to get alarms, the driver needs to be extended to return all
the pages.

    Andrew

^ permalink raw reply

* Re: [PATCH v2 bpf-next] bpf: test_bpf: add init_net to dev for flow_dissector
From: Eric Dumazet @ 2018-09-27 16:39 UTC (permalink / raw)
  To: Song Liu, netdev; +Cc: daniel, kernel-team, Willem de Bruijn, Petar Penkov
In-Reply-To: <20180927163441.3918751-1-songliubraving@fb.com>



On 09/27/2018 09:34 AM, Song Liu wrote:
> Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
> However, this is not true for test_bpf. As a result, test_bpf.ko crashes
> the system with the following stack trace:
> 
> This patch fixes tes_bpf by using init_net in the dummy dev.
> 
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Petar Penkov <ppenkov@google.com>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

^ permalink raw reply

* Re: [PATCH] netfilter: check if the socket netns is correct.
From: Flavio Leitner @ 2018-09-27 22:58 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <20180927204629.GA4680@roeck-us.net>

On Thu, Sep 27, 2018 at 01:46:29PM -0700, Guenter Roeck wrote:
> Hi Flavio,
> 
> On Wed, Jun 27, 2018 at 10:34:25AM -0300, Flavio Leitner wrote:
> > Netfilter assumes that if the socket is present in the skb, then
> > it can be used because that reference is cleaned up while the skb
> > is crossing netns.
> > 
> > We want to change that to preserve the socket reference in a future
> > patch, so this is a preparation updating netfilter to check if the
> > socket netns matches before use it.
> > 
> > Signed-off-by: Flavio Leitner <fbl@redhat.com>
> > Acked-by: Florian Westphal <fw@strlen.de>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> > ---
> ...
> > --- a/net/netfilter/xt_socket.c
> > +++ b/net/netfilter/xt_socket.c
> > @@ -56,8 +56,12 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
> >  	struct sk_buff *pskb = (struct sk_buff *)skb;
> >  	struct sock *sk = skb->sk;
> >  
> > +	if (!net_eq(xt_net(par), sock_net(sk)))
> > +		sk = NULL;
> > +
> 
> I am having trouble with this code. With CONFIG_NET_NS enabled, it crashes
> for me in read_pnet() because sk is NULL.
> 
> >  	if (!sk)
> >  		sk = nf_sk_lookup_slow_v4(xt_net(par), skb, xt_in(par));
> 
> The old code seems to suggest that sk == NULL was possible.
> 
> I see the problem with the Chrome OS kernel rebased to v4.19-rc5, so I
> can not guarantee that this really an upstream problem. The change seems
> odd, though. Are you sure that it is not (or, rather, no longer) necessary
> to check if sk == NULL before dereferencing it in sock_net() ?

Oops, it is necessary but if it's not and the netns doesn't match, we need
do the lookup. So, could you check if this fixes the problem for you?

>From a5f927e7f1368d753f87cb978d630d786d5adb62 Mon Sep 17 00:00:00 2001
From: Flavio Leitner <fbl@redhat.com>
Date: Thu, 27 Sep 2018 19:36:28 -0300
Subject: [PATCH] xt_socket: check sk before checking for netns.

Only check for the network namespace if the socket is available.

Fixes: f564650106a6 ("netfilter: check if the socket netns is correct.")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 net/netfilter/xt_socket.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
index 0472f3472842..ada144e5645b 100644
--- a/net/netfilter/xt_socket.c
+++ b/net/netfilter/xt_socket.c
@@ -56,7 +56,7 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
 	struct sk_buff *pskb = (struct sk_buff *)skb;
 	struct sock *sk = skb->sk;
 
-	if (!net_eq(xt_net(par), sock_net(sk)))
+	if (sk && !net_eq(xt_net(par), sock_net(sk)))
 		sk = NULL;
 
 	if (!sk)
@@ -117,7 +117,7 @@ socket_mt6_v1_v2_v3(const struct sk_buff *skb, struct xt_action_param *par)
 	struct sk_buff *pskb = (struct sk_buff *)skb;
 	struct sock *sk = skb->sk;
 
-	if (!net_eq(xt_net(par), sock_net(sk)))
+	if (sk && !net_eq(xt_net(par), sock_net(sk)))
 		sk = NULL;
 
 	if (!sk)
-- 
2.14.4

^ permalink raw reply related

* Re: [PATCH v2 bpf-next] bpf: test_bpf: add init_net to dev for flow_dissector
From: Willem de Bruijn @ 2018-09-27 16:42 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: songliubraving, Network Development, Daniel Borkmann, Kernel Team,
	Willem de Bruijn, Petar Penkov
In-Reply-To: <f83d10c4-b5fd-0223-e1b6-d376751c3da2@gmail.com>

On Thu, Sep 27, 2018 at 12:40 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/27/2018 09:34 AM, Song Liu wrote:
> > Latest changes in __skb_flow_dissect() assume skb->dev has valid nd_net.
> > However, this is not true for test_bpf. As a result, test_bpf.ko crashes
> > the system with the following stack trace:
> >
> > This patch fixes tes_bpf by using init_net in the dummy dev.
> >
> > Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> > Reported-by: Eric Dumazet <edumazet@google.com>
> > Cc: Willem de Bruijn <willemb@google.com>
> > Cc: Petar Penkov <ppenkov@google.com>
> > Signed-off-by: Song Liu <songliubraving@fb.com>
> > ---
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>

Acked-by: Willem de Bruijn <willemb@google.com>

Thanks!

^ permalink raw reply

* Re: bpf: Massive skbuff_head_cache memory leak?
From: Dmitry Vyukov @ 2018-09-27 16:47 UTC (permalink / raw)
  To: Eric Dumazet, Tetsuo Handa
  Cc: Daniel Borkmann, Alexei Starovoitov, Network Development,
	David S. Miller, Andrew Morton, Michal Hocko, John Johansen,
	syzkaller
In-Reply-To: <CACT4Y+Y8==5_89uoTk1Ryqhj+pCxbv+KsF9oDUvEp=ch3VHdeg@mail.gmail.com>

On Thu, Sep 27, 2018 at 12:27 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Sep 27, 2018 at 1:35 AM, John Johansen
> <john.johansen@canonical.com> wrote:
>> On 09/26/2018 02:22 PM, Daniel Borkmann wrote:
>>> On 09/26/2018 11:09 PM, Tetsuo Handa wrote:
>>>> Hello, Alexei and Daniel.
>>>>
>>>> Can you show us how to run testcases you are testing?
>>>
>>> Sorry for the delay; currently quite backlogged but will definitely take a look
>>> at these reports. Regarding your question: majority of test cases are in the
>>> kernel tree under selftests, see tools/testing/selftests/bpf/ .
>>>
>>
>> Its unlikely to be apparmor. I went through the reports and saw nothing that
>> would indicate apparmor involvement, but the primary reason is what is being tested
>> in upstream apparmor atm.
>>
>> The current upstream code does nothing directly with skbuffs. Its
>> possible that the audit code paths (kernel audit does grab skbuffs)
>> could, but there are only a couple cases that would be triggered in
>> the current fuzzing so this seems to be an unlikely source for such a
>> large leak.
>
>
> Ack. There is no direct evidence against apparmor, I am just trying to
> get at least some hooks re the root cause.
>
> From all the weak indirect evidence, I leaning towards skb allocation
> in an infinite loop (or a timer with infinite rate).
>
>>>> On 2018/09/22 22:25, Tetsuo Handa wrote:
>>>>> Hello.
>>>>>
>>>>> syzbot is reporting many lockup problems on bpf.git / bpf-next.git / net.git / net-next.git trees.
>>>>>
>>>>>   INFO: rcu detected stall in br_multicast_port_group_expired (2)
>>>>>   https://syzkaller.appspot.com/bug?id=15c7ad8cf35a07059e8a697a22527e11d294bc94
>>>>>
>>>>>   INFO: rcu detected stall in tun_chr_close
>>>>>   https://syzkaller.appspot.com/bug?id=6c50618bde03e5a2eefdd0269cf9739c5ebb8270
>>>>>
>>>>>   INFO: rcu detected stall in discover_timer
>>>>>   https://syzkaller.appspot.com/bug?id=55da031ddb910e58ab9c6853a5784efd94f03b54
>>>>>
>>>>>   INFO: rcu detected stall in ret_from_fork (2)
>>>>>   https://syzkaller.appspot.com/bug?id=c83129a6683b44b39f5b8864a1325893c9218363
>>>>>
>>>>>   INFO: rcu detected stall in addrconf_rs_timer
>>>>>   https://syzkaller.appspot.com/bug?id=21c029af65f81488edbc07a10ed20792444711b6
>>>>>
>>>>>   INFO: rcu detected stall in kthread (2)
>>>>>   https://syzkaller.appspot.com/bug?id=6accd1ed11c31110fed1982f6ad38cc9676477d2
>>>>>
>>>>>   INFO: rcu detected stall in ext4_filemap_fault
>>>>>   https://syzkaller.appspot.com/bug?id=817e38d20e9ee53390ac361bf0fd2007eaf188af
>>>>>
>>>>>   INFO: rcu detected stall in run_timer_softirq (2)
>>>>>   https://syzkaller.appspot.com/bug?id=f5a230a3ff7822f8d39fddf8485931bd06ae47fe
>>>>>
>>>>>   INFO: rcu detected stall in bpf_prog_ADDR
>>>>>   https://syzkaller.appspot.com/bug?id=fb4911fd0e861171cc55124e209f810a0dd68744
>>>>>
>>>>>   INFO: rcu detected stall in __run_timers (2)
>>>>>   https://syzkaller.appspot.com/bug?id=65416569ddc8d2feb8f19066aa761f5a47f7451a
>>>>>
>>>>> The cause of lockup seems to be flood of printk() messages from memory allocation
>>>>> failures, and one of out_of_memory() messages indicates that skbuff_head_cache
>>>>> usage is huge enough to suspect in-kernel memory leaks.
>>>>>
>>>>>   [ 1554.547011] skbuff_head_cache    1847887KB    1847887KB
>>>>>
>>>>> Unfortunately, we cannot find from logs what syzbot is trying to do
>>>>> because constant printk() messages is flooding away syzkaller messages.
>>>>> Can you try running your testcases with kmemleak enabled?
>>>>>
>>>>
>>>> On 2018/09/27 2:35, Dmitry Vyukov wrote:
>>>>> I also started suspecting Apparmor. We switched to Apparmor on Aug 30:
>>>>> https://groups.google.com/d/msg/syzkaller-bugs/o73lO4KGh0w/j9pcH2tSBAAJ
>>>>> Now the instances that use SELinux and Smack explicitly contain that
>>>>> in the name, but the rest are Apparmor.
>>>>> Aug 30 roughly matches these assorted "task hung" reports. Perhaps
>>>>> some Apparmor hook leaks a reference to skbs?
>>>>
>>>> Maybe. They have CONFIG_DEFAULT_SECURITY="apparmor". But I'm wondering why
>>>> this problem is not occurring on linux-next.git when this problem is occurring
>>>> on bpf.git / bpf-next.git / net.git / net-next.git trees. Is syzbot running
>>>> different testcases depending on which git tree is targeted?
>>>>
>> this is another reason that it is doubtful that its apparmor.

On Thu, Sep 27, 2018 at 2:52 PM, edumazet
> Have you tried kmemleak perhaps, it might give us a clue, but it seems
> obvious the leak would be in TX path.

So, I've tried. Now what? :)

I've uploaded all reports to:
https://drive.google.com/file/d/107LUW0zmYbXmxfQCWoLpeenxXJsXIkxj/view?usp=sharing
This is on net tree d4ce58082f206bf6e7d697380c7bc5480a8b0264

memory leak in __lookup_hash    33    Sep 27 2018 16:35:50
memory leak in new_inode_pseudo    43    Sep 27 2018 16:41:14
memory leak in path_openat    1    Sep 27 2018 16:12:53
memory leak in rhashtable_init    1    Sep 27 2018 16:41:41
memory leak in shmem_symlink    4    Sep 27 2018 16:34:34
memory leak in __anon_vma_prepare    1    Sep 27 2018 17:30:02
memory leak in __do_execve_file    4    Sep 27 2018 18:14:10
memory leak in __do_sys_perf_event_open    2    Sep 27 2018 17:40:05
memory leak in __es_insert_extent    3    Sep 27 2018 17:24:52
memory leak in __getblk_gfp    3    Sep 27 2018 18:19:30
memory leak in __handle_mm_fault    1    Sep 27 2018 18:11:31
memory leak in __hw_addr_create_ex    2    Sep 27 2018 18:10:56
memory leak in __ip_mc_inc_group    13    Sep 27 2018 18:24:31
memory leak in __khugepaged_enter    1    Sep 27 2018 15:40:25
memory leak in __list_lru_init    27    Sep 27 2018 17:30:48
memory leak in __neigh_create    12    Sep 27 2018 17:40:28
memory leak in __netlink_create    1    Sep 27 2018 15:40:23
memory leak in __register_sysctl_table    1    Sep 27 2018 17:36:57
memory leak in __send_signal    1    Sep 27 2018 18:30:48
memory leak in __sys_socket    7    Sep 27 2018 15:43:20
memory leak in anon_inode_getfile    4    Sep 27 2018 18:17:29
memory leak in bpf_prog_store_orig_filter    1    Sep 27 2018 17:59:14
memory leak in br_multicast_new_group    2    Sep 27 2018 18:16:42
memory leak in br_multicast_new_port_group    3    Sep 27 2018 18:17:39
memory leak in build_sched_domains    2    Sep 27 2018 17:28:55
memory leak in clone_mnt    2    Sep 27 2018 18:18:48
memory leak in compute_effective_progs    1    Sep 27 2018 18:35:15
memory leak in create_empty_buffers    1    Sep 27 2018 17:35:42
memory leak in create_filter_start    2    Sep 27 2018 18:16:00
memory leak in create_pipe_files    3    Sep 27 2018 18:21:47
memory leak in do_ip6t_set_ctl    1    Sep 27 2018 15:40:21
memory leak in do_ipt_set_ctl    1    Sep 27 2018 17:37:35
memory leak in do_signalfd4    1    Sep 27 2018 18:00:45
memory leak in do_syslog    2    Sep 27 2018 18:06:22
memory leak in ep_insert    11    Sep 27 2018 17:39:14
memory leak in ep_ptable_queue_proc    3    Sep 27 2018 17:37:17
memory leak in ext4_mb_new_group_pa    1    Sep 27 2018 17:36:04
memory leak in ext4_mb_new_inode_pa    3    Sep 27 2018 17:39:19
memory leak in fdb_create    13    Sep 27 2018 18:24:03
memory leak in fib6_add_1    11    Sep 27 2018 18:19:53
memory leak in fib_table_insert    1    Sep 27 2018 17:36:49
memory leak in find_get_context    1    Sep 27 2018 15:42:00
memory leak in fsnotify_add_mark_locked    11    Sep 27 2018 17:31:41
memory leak in idr_get_free    2    Sep 27 2018 18:17:34
memory leak in iget_locked    1    Sep 27 2018 17:52:59
memory leak in inet_frag_find    3    Sep 27 2018 18:23:53
memory leak in inotify_update_watch    25    Sep 27 2018 17:30:59
memory leak in ioc_create_icq    1    Sep 27 2018 15:42:40
memory leak in ip6_pol_route    4    Sep 27 2018 18:24:59
memory leak in ip6_route_info_create    1    Sep 27 2018 17:38:09
memory leak in ip6t_register_table    1    Sep 27 2018 17:35:39
memory leak in ip_route_output_key_hash_rcu    4    Sep 27 2018 18:21:21
memory leak in ipt_register_table    3    Sep 27 2018 17:39:58
memory leak in ipv6_add_addr    1    Sep 27 2018 17:38:49
memory leak in load_elf_binary    1    Sep 27 2018 18:09:44
memory leak in map_create    9    Sep 27 2018 18:25:10
memory leak in memcg_update_all_list_lrus    1    Sep 27 2018 15:39:36
memory leak in ndisc_send_rs    1    Sep 27 2018 17:50:03
memory leak in neigh_table_init    6    Sep 27 2018 17:23:41
memory leak in nf_hook_entries_grow    5    Sep 27 2018 15:43:41
memory leak in packet_sendmsg    1    Sep 27 2018 18:08:43
memory leak in pcpu_create_chunk    1    Sep 27 2018 17:48:41
memory leak in prepare_creds    18    Sep 27 2018 17:29:10
memory leak in prepare_kernel_cred    15    Sep 27 2018 18:23:42
memory leak in process_preds    2    Sep 27 2018 18:11:01
memory leak in rht_deferred_worker    9    Sep 27 2018 18:24:29
memory leak in sched_init_domains    2    Sep 27 2018 18:12:42
memory leak in sctp_addr_wq_mgmt    1    Sep 27 2018 17:46:20
memory leak in sget    5    Sep 27 2018 18:03:40
memory leak in shmem_symlink    30    Sep 27 2018 17:32:09
memory leak in skb_clone    3    Sep 27 2018 18:13:22
memory leak in submit_bh_wbc    1    Sep 27 2018 17:49:06
memory leak in tracepoint_probe_register_prio    1    Sep 27 2018 17:39:13
memory leak in xt_replace_table    4    Sep 27 2018 15:43:19
memory leak in __delayacct_tsk_init    2    Sep 27 2018 17:02:53
memory leak in disk_expand_part_tbl    1    Sep 27 2018 16:59:05
memory leak in do_ip6t_set_ctl    14    Sep 27 2018 15:46:37
memory leak in neigh_table_init    4    Sep 27 2018 17:10:29
memory leak in do_check    1    Sep 27 2018 18:38:13

^ permalink raw reply

* Re: [PATCH] netfilter: check if the socket netns is correct.
From: Guenter Roeck @ 2018-09-27 23:08 UTC (permalink / raw)
  To: Flavio Leitner
  Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
	David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
	netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <20180927225824.GT2702@plex.lan>

On Thu, Sep 27, 2018 at 07:58:24PM -0300, Flavio Leitner wrote:
> On Thu, Sep 27, 2018 at 01:46:29PM -0700, Guenter Roeck wrote:
> > Hi Flavio,
> > 
> > On Wed, Jun 27, 2018 at 10:34:25AM -0300, Flavio Leitner wrote:
> > > Netfilter assumes that if the socket is present in the skb, then
> > > it can be used because that reference is cleaned up while the skb
> > > is crossing netns.
> > > 
> > > We want to change that to preserve the socket reference in a future
> > > patch, so this is a preparation updating netfilter to check if the
> > > socket netns matches before use it.
> > > 
> > > Signed-off-by: Flavio Leitner <fbl@redhat.com>
> > > Acked-by: Florian Westphal <fw@strlen.de>
> > > Signed-off-by: David S. Miller <davem@davemloft.net>
> > > ---
> > ...
> > > --- a/net/netfilter/xt_socket.c
> > > +++ b/net/netfilter/xt_socket.c
> > > @@ -56,8 +56,12 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
> > >  	struct sk_buff *pskb = (struct sk_buff *)skb;
> > >  	struct sock *sk = skb->sk;
> > >  
> > > +	if (!net_eq(xt_net(par), sock_net(sk)))
> > > +		sk = NULL;
> > > +
> > 
> > I am having trouble with this code. With CONFIG_NET_NS enabled, it crashes
> > for me in read_pnet() because sk is NULL.
> > 
> > >  	if (!sk)
> > >  		sk = nf_sk_lookup_slow_v4(xt_net(par), skb, xt_in(par));
> > 
> > The old code seems to suggest that sk == NULL was possible.
> > 
> > I see the problem with the Chrome OS kernel rebased to v4.19-rc5, so I
> > can not guarantee that this really an upstream problem. The change seems
> > odd, though. Are you sure that it is not (or, rather, no longer) necessary
> > to check if sk == NULL before dereferencing it in sock_net() ?
> 
> Oops, it is necessary but if it's not and the netns doesn't match, we need
> do the lookup. So, could you check if this fixes the problem for you?
> 
> From a5f927e7f1368d753f87cb978d630d786d5adb62 Mon Sep 17 00:00:00 2001
> From: Flavio Leitner <fbl@redhat.com>
> Date: Thu, 27 Sep 2018 19:36:28 -0300
> Subject: [PATCH] xt_socket: check sk before checking for netns.
> 
> Only check for the network namespace if the socket is available.
> 
> Fixes: f564650106a6 ("netfilter: check if the socket netns is correct.")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Flavio Leitner <fbl@redhat.com>

This fixes the problem for me.

Tested-by: Guenter Roeck <linux@roeck-us.net>

Thanks,
Guenter

> ---
>  net/netfilter/xt_socket.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/xt_socket.c b/net/netfilter/xt_socket.c
> index 0472f3472842..ada144e5645b 100644
> --- a/net/netfilter/xt_socket.c
> +++ b/net/netfilter/xt_socket.c
> @@ -56,7 +56,7 @@ socket_match(const struct sk_buff *skb, struct xt_action_param *par,
>  	struct sk_buff *pskb = (struct sk_buff *)skb;
>  	struct sock *sk = skb->sk;
>  
> -	if (!net_eq(xt_net(par), sock_net(sk)))
> +	if (sk && !net_eq(xt_net(par), sock_net(sk)))
>  		sk = NULL;
>  
>  	if (!sk)
> @@ -117,7 +117,7 @@ socket_mt6_v1_v2_v3(const struct sk_buff *skb, struct xt_action_param *par)
>  	struct sk_buff *pskb = (struct sk_buff *)skb;
>  	struct sock *sk = skb->sk;
>  
> -	if (!net_eq(xt_net(par), sock_net(sk)))
> +	if (sk && !net_eq(xt_net(par), sock_net(sk)))
>  		sk = NULL;
>  
>  	if (!sk)
> -- 
> 2.14.4
> 

^ permalink raw reply

* Re: bpf: Massive skbuff_head_cache memory leak?
From: Dmitry Vyukov @ 2018-09-27 16:53 UTC (permalink / raw)
  To: Eric Dumazet, Tetsuo Handa
  Cc: Daniel Borkmann, Alexei Starovoitov, Network Development,
	David S. Miller, Andrew Morton, Michal Hocko, John Johansen,
	syzkaller
In-Reply-To: <CACT4Y+Zw9y+EFDkZTWKKynPg-3c4_iUHaptCn16t9pBQx9d6bQ@mail.gmail.com>

On Thu, Sep 27, 2018 at 6:47 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Sep 27, 2018 at 12:27 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> On Thu, Sep 27, 2018 at 1:35 AM, John Johansen
>> <john.johansen@canonical.com> wrote:
>>> On 09/26/2018 02:22 PM, Daniel Borkmann wrote:
>>>> On 09/26/2018 11:09 PM, Tetsuo Handa wrote:
>>>>> Hello, Alexei and Daniel.
>>>>>
>>>>> Can you show us how to run testcases you are testing?
>>>>
>>>> Sorry for the delay; currently quite backlogged but will definitely take a look
>>>> at these reports. Regarding your question: majority of test cases are in the
>>>> kernel tree under selftests, see tools/testing/selftests/bpf/ .
>>>>
>>>
>>> Its unlikely to be apparmor. I went through the reports and saw nothing that
>>> would indicate apparmor involvement, but the primary reason is what is being tested
>>> in upstream apparmor atm.
>>>
>>> The current upstream code does nothing directly with skbuffs. Its
>>> possible that the audit code paths (kernel audit does grab skbuffs)
>>> could, but there are only a couple cases that would be triggered in
>>> the current fuzzing so this seems to be an unlikely source for such a
>>> large leak.
>>
>>
>> Ack. There is no direct evidence against apparmor, I am just trying to
>> get at least some hooks re the root cause.
>>
>> From all the weak indirect evidence, I leaning towards skb allocation
>> in an infinite loop (or a timer with infinite rate).
>>
>>>>> On 2018/09/22 22:25, Tetsuo Handa wrote:
>>>>>> Hello.
>>>>>>
>>>>>> syzbot is reporting many lockup problems on bpf.git / bpf-next.git / net.git / net-next.git trees.
>>>>>>
>>>>>>   INFO: rcu detected stall in br_multicast_port_group_expired (2)
>>>>>>   https://syzkaller.appspot.com/bug?id=15c7ad8cf35a07059e8a697a22527e11d294bc94
>>>>>>
>>>>>>   INFO: rcu detected stall in tun_chr_close
>>>>>>   https://syzkaller.appspot.com/bug?id=6c50618bde03e5a2eefdd0269cf9739c5ebb8270
>>>>>>
>>>>>>   INFO: rcu detected stall in discover_timer
>>>>>>   https://syzkaller.appspot.com/bug?id=55da031ddb910e58ab9c6853a5784efd94f03b54
>>>>>>
>>>>>>   INFO: rcu detected stall in ret_from_fork (2)
>>>>>>   https://syzkaller.appspot.com/bug?id=c83129a6683b44b39f5b8864a1325893c9218363
>>>>>>
>>>>>>   INFO: rcu detected stall in addrconf_rs_timer
>>>>>>   https://syzkaller.appspot.com/bug?id=21c029af65f81488edbc07a10ed20792444711b6
>>>>>>
>>>>>>   INFO: rcu detected stall in kthread (2)
>>>>>>   https://syzkaller.appspot.com/bug?id=6accd1ed11c31110fed1982f6ad38cc9676477d2
>>>>>>
>>>>>>   INFO: rcu detected stall in ext4_filemap_fault
>>>>>>   https://syzkaller.appspot.com/bug?id=817e38d20e9ee53390ac361bf0fd2007eaf188af
>>>>>>
>>>>>>   INFO: rcu detected stall in run_timer_softirq (2)
>>>>>>   https://syzkaller.appspot.com/bug?id=f5a230a3ff7822f8d39fddf8485931bd06ae47fe
>>>>>>
>>>>>>   INFO: rcu detected stall in bpf_prog_ADDR
>>>>>>   https://syzkaller.appspot.com/bug?id=fb4911fd0e861171cc55124e209f810a0dd68744
>>>>>>
>>>>>>   INFO: rcu detected stall in __run_timers (2)
>>>>>>   https://syzkaller.appspot.com/bug?id=65416569ddc8d2feb8f19066aa761f5a47f7451a
>>>>>>
>>>>>> The cause of lockup seems to be flood of printk() messages from memory allocation
>>>>>> failures, and one of out_of_memory() messages indicates that skbuff_head_cache
>>>>>> usage is huge enough to suspect in-kernel memory leaks.
>>>>>>
>>>>>>   [ 1554.547011] skbuff_head_cache    1847887KB    1847887KB
>>>>>>
>>>>>> Unfortunately, we cannot find from logs what syzbot is trying to do
>>>>>> because constant printk() messages is flooding away syzkaller messages.
>>>>>> Can you try running your testcases with kmemleak enabled?
>>>>>>
>>>>>
>>>>> On 2018/09/27 2:35, Dmitry Vyukov wrote:
>>>>>> I also started suspecting Apparmor. We switched to Apparmor on Aug 30:
>>>>>> https://groups.google.com/d/msg/syzkaller-bugs/o73lO4KGh0w/j9pcH2tSBAAJ
>>>>>> Now the instances that use SELinux and Smack explicitly contain that
>>>>>> in the name, but the rest are Apparmor.
>>>>>> Aug 30 roughly matches these assorted "task hung" reports. Perhaps
>>>>>> some Apparmor hook leaks a reference to skbs?
>>>>>
>>>>> Maybe. They have CONFIG_DEFAULT_SECURITY="apparmor". But I'm wondering why
>>>>> this problem is not occurring on linux-next.git when this problem is occurring
>>>>> on bpf.git / bpf-next.git / net.git / net-next.git trees. Is syzbot running
>>>>> different testcases depending on which git tree is targeted?
>>>>>
>>> this is another reason that it is doubtful that its apparmor.
>
> On Thu, Sep 27, 2018 at 2:52 PM, edumazet
>> Have you tried kmemleak perhaps, it might give us a clue, but it seems
>> obvious the leak would be in TX path.
>
> So, I've tried. Now what? :)
>
> I've uploaded all reports to:
> https://drive.google.com/file/d/107LUW0zmYbXmxfQCWoLpeenxXJsXIkxj/view?usp=sharing
> This is on net tree d4ce58082f206bf6e7d697380c7bc5480a8b0264
>
> memory leak in __lookup_hash    33    Sep 27 2018 16:35:50
> memory leak in new_inode_pseudo    43    Sep 27 2018 16:41:14
> memory leak in path_openat    1    Sep 27 2018 16:12:53
> memory leak in rhashtable_init    1    Sep 27 2018 16:41:41
> memory leak in shmem_symlink    4    Sep 27 2018 16:34:34
> memory leak in __anon_vma_prepare    1    Sep 27 2018 17:30:02
> memory leak in __do_execve_file    4    Sep 27 2018 18:14:10
> memory leak in __do_sys_perf_event_open    2    Sep 27 2018 17:40:05
> memory leak in __es_insert_extent    3    Sep 27 2018 17:24:52
> memory leak in __getblk_gfp    3    Sep 27 2018 18:19:30
> memory leak in __handle_mm_fault    1    Sep 27 2018 18:11:31
> memory leak in __hw_addr_create_ex    2    Sep 27 2018 18:10:56
> memory leak in __ip_mc_inc_group    13    Sep 27 2018 18:24:31
> memory leak in __khugepaged_enter    1    Sep 27 2018 15:40:25
> memory leak in __list_lru_init    27    Sep 27 2018 17:30:48
> memory leak in __neigh_create    12    Sep 27 2018 17:40:28
> memory leak in __netlink_create    1    Sep 27 2018 15:40:23
> memory leak in __register_sysctl_table    1    Sep 27 2018 17:36:57
> memory leak in __send_signal    1    Sep 27 2018 18:30:48
> memory leak in __sys_socket    7    Sep 27 2018 15:43:20
> memory leak in anon_inode_getfile    4    Sep 27 2018 18:17:29
> memory leak in bpf_prog_store_orig_filter    1    Sep 27 2018 17:59:14
> memory leak in br_multicast_new_group    2    Sep 27 2018 18:16:42
> memory leak in br_multicast_new_port_group    3    Sep 27 2018 18:17:39
> memory leak in build_sched_domains    2    Sep 27 2018 17:28:55
> memory leak in clone_mnt    2    Sep 27 2018 18:18:48
> memory leak in compute_effective_progs    1    Sep 27 2018 18:35:15
> memory leak in create_empty_buffers    1    Sep 27 2018 17:35:42
> memory leak in create_filter_start    2    Sep 27 2018 18:16:00
> memory leak in create_pipe_files    3    Sep 27 2018 18:21:47
> memory leak in do_ip6t_set_ctl    1    Sep 27 2018 15:40:21
> memory leak in do_ipt_set_ctl    1    Sep 27 2018 17:37:35
> memory leak in do_signalfd4    1    Sep 27 2018 18:00:45
> memory leak in do_syslog    2    Sep 27 2018 18:06:22
> memory leak in ep_insert    11    Sep 27 2018 17:39:14
> memory leak in ep_ptable_queue_proc    3    Sep 27 2018 17:37:17
> memory leak in ext4_mb_new_group_pa    1    Sep 27 2018 17:36:04
> memory leak in ext4_mb_new_inode_pa    3    Sep 27 2018 17:39:19
> memory leak in fdb_create    13    Sep 27 2018 18:24:03
> memory leak in fib6_add_1    11    Sep 27 2018 18:19:53
> memory leak in fib_table_insert    1    Sep 27 2018 17:36:49
> memory leak in find_get_context    1    Sep 27 2018 15:42:00
> memory leak in fsnotify_add_mark_locked    11    Sep 27 2018 17:31:41
> memory leak in idr_get_free    2    Sep 27 2018 18:17:34
> memory leak in iget_locked    1    Sep 27 2018 17:52:59
> memory leak in inet_frag_find    3    Sep 27 2018 18:23:53
> memory leak in inotify_update_watch    25    Sep 27 2018 17:30:59
> memory leak in ioc_create_icq    1    Sep 27 2018 15:42:40
> memory leak in ip6_pol_route    4    Sep 27 2018 18:24:59
> memory leak in ip6_route_info_create    1    Sep 27 2018 17:38:09
> memory leak in ip6t_register_table    1    Sep 27 2018 17:35:39
> memory leak in ip_route_output_key_hash_rcu    4    Sep 27 2018 18:21:21
> memory leak in ipt_register_table    3    Sep 27 2018 17:39:58
> memory leak in ipv6_add_addr    1    Sep 27 2018 17:38:49
> memory leak in load_elf_binary    1    Sep 27 2018 18:09:44
> memory leak in map_create    9    Sep 27 2018 18:25:10
> memory leak in memcg_update_all_list_lrus    1    Sep 27 2018 15:39:36
> memory leak in ndisc_send_rs    1    Sep 27 2018 17:50:03
> memory leak in neigh_table_init    6    Sep 27 2018 17:23:41
> memory leak in nf_hook_entries_grow    5    Sep 27 2018 15:43:41
> memory leak in packet_sendmsg    1    Sep 27 2018 18:08:43
> memory leak in pcpu_create_chunk    1    Sep 27 2018 17:48:41
> memory leak in prepare_creds    18    Sep 27 2018 17:29:10
> memory leak in prepare_kernel_cred    15    Sep 27 2018 18:23:42
> memory leak in process_preds    2    Sep 27 2018 18:11:01
> memory leak in rht_deferred_worker    9    Sep 27 2018 18:24:29
> memory leak in sched_init_domains    2    Sep 27 2018 18:12:42
> memory leak in sctp_addr_wq_mgmt    1    Sep 27 2018 17:46:20
> memory leak in sget    5    Sep 27 2018 18:03:40
> memory leak in shmem_symlink    30    Sep 27 2018 17:32:09
> memory leak in skb_clone    3    Sep 27 2018 18:13:22
> memory leak in submit_bh_wbc    1    Sep 27 2018 17:49:06
> memory leak in tracepoint_probe_register_prio    1    Sep 27 2018 17:39:13
> memory leak in xt_replace_table    4    Sep 27 2018 15:43:19
> memory leak in __delayacct_tsk_init    2    Sep 27 2018 17:02:53
> memory leak in disk_expand_part_tbl    1    Sep 27 2018 16:59:05
> memory leak in do_ip6t_set_ctl    14    Sep 27 2018 15:46:37
> memory leak in neigh_table_init    4    Sep 27 2018 17:10:29
> memory leak in do_check    1    Sep 27 2018 18:38:13


I see at least 3 bridge-related:
memory leak in skb_clone
memory leak in br_multicast_new_group
memory leak in br_multicast_new_port_group

^ permalink raw reply

* [PATCH] qed: fix spelling mistake "b_cb_registred" -> "b_cb_registered"
From: Colin King @ 2018-09-27 17:04 UTC (permalink / raw)
  To: Ariel Elior, everest-linux-l2, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

Trivial fix to spelling mistake struct field name, rename it.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/ethernet/qlogic/qed/qed_ll2.c | 12 ++++++------
 drivers/net/ethernet/qlogic/qed/qed_ll2.h |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
index 14ac9cab2653..da13117a604a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c
@@ -63,8 +63,8 @@
 #include "qed_sp.h"
 #include "qed_rdma.h"
 
-#define QED_LL2_RX_REGISTERED(ll2)	((ll2)->rx_queue.b_cb_registred)
-#define QED_LL2_TX_REGISTERED(ll2)	((ll2)->tx_queue.b_cb_registred)
+#define QED_LL2_RX_REGISTERED(ll2)	((ll2)->rx_queue.b_cb_registered)
+#define QED_LL2_TX_REGISTERED(ll2)	((ll2)->tx_queue.b_cb_registered)
 
 #define QED_LL2_TX_SIZE (256)
 #define QED_LL2_RX_SIZE (4096)
@@ -1404,7 +1404,7 @@ int qed_ll2_acquire_connection(void *cxt, struct qed_ll2_acquire_data *data)
 				    &p_hwfn->p_ll2_info[i],
 				    &p_ll2_info->rx_queue.rx_sb_index,
 				    &p_ll2_info->rx_queue.p_fw_cons);
-		p_ll2_info->rx_queue.b_cb_registred = true;
+		p_ll2_info->rx_queue.b_cb_registered = true;
 	}
 
 	if (data->input.tx_num_desc) {
@@ -1413,7 +1413,7 @@ int qed_ll2_acquire_connection(void *cxt, struct qed_ll2_acquire_data *data)
 				    &p_hwfn->p_ll2_info[i],
 				    &p_ll2_info->tx_queue.tx_sb_index,
 				    &p_ll2_info->tx_queue.p_fw_cons);
-		p_ll2_info->tx_queue.b_cb_registred = true;
+		p_ll2_info->tx_queue.b_cb_registered = true;
 	}
 
 	*data->p_connection_handle = i;
@@ -1929,7 +1929,7 @@ int qed_ll2_terminate_connection(void *cxt, u8 connection_handle)
 
 	/* Stop Tx & Rx of connection, if needed */
 	if (QED_LL2_TX_REGISTERED(p_ll2_conn)) {
-		p_ll2_conn->tx_queue.b_cb_registred = false;
+		p_ll2_conn->tx_queue.b_cb_registered = false;
 		smp_wmb(); /* Make sure this is seen by ll2_lb_rxq_completion */
 		rc = qed_sp_ll2_tx_queue_stop(p_hwfn, p_ll2_conn);
 		if (rc)
@@ -1940,7 +1940,7 @@ int qed_ll2_terminate_connection(void *cxt, u8 connection_handle)
 	}
 
 	if (QED_LL2_RX_REGISTERED(p_ll2_conn)) {
-		p_ll2_conn->rx_queue.b_cb_registred = false;
+		p_ll2_conn->rx_queue.b_cb_registered = false;
 		smp_wmb(); /* Make sure this is seen by ll2_lb_rxq_completion */
 		rc = qed_sp_ll2_rx_queue_stop(p_hwfn, p_ll2_conn);
 		if (rc)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.h b/drivers/net/ethernet/qlogic/qed/qed_ll2.h
index f65817012e97..1a5c1ae01474 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_ll2.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.h
@@ -79,7 +79,7 @@ struct qed_ll2_rx_queue {
 	struct qed_chain rxq_chain;
 	struct qed_chain rcq_chain;
 	u8 rx_sb_index;
-	bool b_cb_registred;
+	bool b_cb_registered;
 	__le16 *p_fw_cons;
 	struct list_head active_descq;
 	struct list_head free_descq;
@@ -93,7 +93,7 @@ struct qed_ll2_tx_queue {
 	spinlock_t lock;
 	struct qed_chain txq_chain;
 	u8 tx_sb_index;
-	bool b_cb_registred;
+	bool b_cb_registered;
 	__le16 *p_fw_cons;
 	struct list_head active_descq;
 	struct list_head free_descq;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH net V2] vhost-vsock: fix use after free
From: Jason Wang @ 2018-09-27 23:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: stefanha, kvm, virtualization, netdev, linux-kernel,
	sergei.shtylyov
In-Reply-To: <20180927123539-mutt-send-email-mst@kernel.org>



On 2018年09月28日 01:04, Michael S. Tsirkin wrote:
> On Thu, Sep 27, 2018 at 08:22:04PM +0800, Jason Wang wrote:
>> The access of vsock is not protected by vhost_vsock_lock. This may
>> lead to use after free since vhost_vsock_dev_release() may free the
>> pointer at the same time.
>>
>> Fix this by holding the lock during the access.
>>
>> Reported-by:syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
>> Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
>> Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
>> Cc: Stefan Hajnoczi<stefanha@redhat.com>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
> Wow is that really the best we can do?

For net/stable, probably yes.

>   A global lock on a data path
> operation?

It's already there, and the patch only increase the critical section.

>   Granted use after free is nasty but Stefan said he sees
> a way to fix it using a per socket refcount. He's on vacation
> until Oct 4 though ...
>

Stefan has acked the pacth, so I think it's ok? We can do optimization 
for -next on top.

Thanks

^ permalink raw reply

* Re: [PATCH net V2] vhost-vsock: fix use after free
From: Michael S. Tsirkin @ 2018-09-27 23:50 UTC (permalink / raw)
  To: Jason Wang
  Cc: stefanha, kvm, virtualization, netdev, linux-kernel,
	sergei.shtylyov
In-Reply-To: <588ed28b-7e4b-9dc8-92ce-d75692836c9e@redhat.com>

On Fri, Sep 28, 2018 at 07:37:37AM +0800, Jason Wang wrote:
> 
> 
> On 2018年09月28日 01:04, Michael S. Tsirkin wrote:
> > On Thu, Sep 27, 2018 at 08:22:04PM +0800, Jason Wang wrote:
> > > The access of vsock is not protected by vhost_vsock_lock. This may
> > > lead to use after free since vhost_vsock_dev_release() may free the
> > > pointer at the same time.
> > > 
> > > Fix this by holding the lock during the access.
> > > 
> > > Reported-by:syzbot+e3e074963495f92a89ed@syzkaller.appspotmail.com
> > > Fixes: 16320f363ae1 ("vhost-vsock: add pkt cancel capability")
> > > Fixes: 433fc58e6bf2 ("VSOCK: Introduce vhost_vsock.ko")
> > > Cc: Stefan Hajnoczi<stefanha@redhat.com>
> > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > Wow is that really the best we can do?
> 
> For net/stable, probably yes.
> 
> >   A global lock on a data path
> > operation?
> 
> It's already there,

&vhost_vsock_lock? were is it takes on data path?

> and the patch only increase the critical section.
> 
> >   Granted use after free is nasty but Stefan said he sees
> > a way to fix it using a per socket refcount. He's on vacation
> > until Oct 4 though ...
> > 
> 
> Stefan has acked the pacth, so I think it's ok? We can do optimization for
> -next on top.
> 
> Thanks


Well on high SMP serializing can drop performance as much as x100 so I'm
not sure it's appropriate - seems to fix a bug but can introduce a
regression. Let's see how does a proper fix look first?

-- 
MST

^ permalink raw reply

* RE: [PATCH] net/ncsi: Add NCSI OEM command for FB Tiogapass
From: Justin.Lee1 @ 2018-09-27 17:46 UTC (permalink / raw)
  To: joel
  Cc: amithash, vijaykhemka, sam, linux-aspeed, openbmc, sdasari,
	netdev, christian
In-Reply-To: <CACPK8XfuXa19dGRqL+Ycdgh1LKmoQNuqGvtrrOgeNyvTZR01kQ@mail.gmail.com>


> Thanks for the overview. We look forward to your patches; please
> include the same cc list as this series.

> I think it makes sense to have some OEM NCSI handing purely in the
> kenrel. This would allow eg. the MAC address of an interface to be
> correct at boot, without requiring userspace to come up first.

> I have also heard stories of temperature sensors over NCSI. Those
> would make sense to be hwmon drivers, which again would mean handling
> them in the kernel.

> Justin, Vijay, can you please list the known NCSI OEM
> commands/extensions that we plan on implementing?

In our implementation, All OEM commands are transparent to Kernel.
We don't plan to add any specific handler in the Kernel. 

Thanks,
Justin

^ permalink raw reply

* [PATCH net-next 0/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
  To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner

Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.

Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].

But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.

The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.

[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166

Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>

Christian Brauner (7):
  rtnetlink: add RTM_GETADDR2
  ipv4: add RTM_GETADDR2
  ipv6: add RTM_GETADDR2
  decnet: add RTM_GETADDR2
  phonet: add RTM_GETADDR2
  selinux: add RTM_GETADDR2
  rtnetlink: enable RTM_GETADDR2

 include/uapi/linux/rtnetlink.h |  3 +++
 net/core/rtnetlink.c           |  1 +
 net/decnet/dn_dev.c            | 25 +++++++++++++++++++++++--
 net/ipv4/devinet.c             | 24 +++++++++++++++++++++---
 net/ipv6/addrconf.c            | 30 ++++++++++++++++++++++++------
 net/phonet/pn_netlink.c        | 25 +++++++++++++++++++++++--
 security/selinux/nlmsgtab.c    |  3 ++-
 7 files changed, 97 insertions(+), 14 deletions(-)

-- 
2.17.1

^ permalink raw reply

* [PATCH net-next 1/7] rtnetlink: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
  To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>

Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.

Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].

But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.

The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.

[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166

Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
 include/uapi/linux/rtnetlink.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 46399367627f..e167f90c3d7a 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -157,6 +157,9 @@ enum {
 	RTM_GETCHAIN,
 #define RTM_GETCHAIN RTM_GETCHAIN
 
+	RTM_GETADDR2 = 106,
+#define RTM_GETADDR2  RTM_GETADDR2
+
 	__RTM_MAX,
 #define RTM_MAX		(((__RTM_MAX + 3) & ~3) - 1)
 };
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 3/7] ipv6: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
  To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>

Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.

Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].

But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.

The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.

[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166

Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
 net/ipv6/addrconf.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index a9a317322388..6e76e5cc76c4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -5003,7 +5003,7 @@ static int in6_dump_addrs(struct inet6_dev *idev, struct sk_buff *skb,
 }
 
 static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
-			   enum addr_type_t type)
+			   enum addr_type_t type, int rtm_version)
 {
 	struct net *net = sock_net(skb->sk);
 	struct nlattr *tb[IFA_MAX+1];
@@ -5020,8 +5020,14 @@ static int inet6_dump_addr(struct sk_buff *skb, struct netlink_callback *cb,
 	s_idx = idx = cb->args[1];
 	s_ip_idx = ip_idx = cb->args[2];
 
-	if (nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
-			ifa_ipv6_policy, NULL) >= 0) {
+	if (rtm_version == RTM_GETADDR2) {
+		int err;
+
+		err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb,
+				  IFA_MAX, ifa_ipv6_policy, NULL);
+		if (err < 0)
+			return err;
+
 		if (tb[IFA_TARGET_NETNSID]) {
 			netnsid = nla_get_s32(tb[IFA_TARGET_NETNSID]);
 
@@ -5068,14 +5074,21 @@ static int inet6_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = UNICAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETADDR);
+}
+
+static int inet6_dump_ifaddr2(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	enum addr_type_t type = UNICAST_ADDR;
+
+	return inet6_dump_addr(skb, cb, type, RTM_GETADDR2);
 }
 
 static int inet6_dump_ifmcaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = MULTICAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETMULTICAST);
 }
 
 
@@ -5083,7 +5096,7 @@ static int inet6_dump_ifacaddr(struct sk_buff *skb, struct netlink_callback *cb)
 {
 	enum addr_type_t type = ANYCAST_ADDR;
 
-	return inet6_dump_addr(skb, cb, type);
+	return inet6_dump_addr(skb, cb, type, RTM_GETANYCAST);
 }
 
 static int inet6_rtm_getaddr(struct sk_buff *in_skb, struct nlmsghdr *nlh,
@@ -6833,6 +6846,11 @@ int __init addrconf_init(void)
 				   RTNL_FLAG_DOIT_UNLOCKED);
 	if (err < 0)
 		goto errout;
+	err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETADDR2,
+				   inet6_rtm_getaddr, inet6_dump_ifaddr2,
+				   RTNL_FLAG_DOIT_UNLOCKED);
+	if (err < 0)
+		goto errout;
 	err = rtnl_register_module(THIS_MODULE, PF_INET6, RTM_GETMULTICAST,
 				   NULL, inet6_dump_ifmcaddr, 0);
 	if (err < 0)
-- 
2.17.1

^ permalink raw reply related

* [PATCH net-next 4/7] decnet: add RTM_GETADDR2
From: Christian Brauner @ 2018-09-27 17:58 UTC (permalink / raw)
  To: jbenc, davem, dsahern, stephen, netdev, linux-kernel; +Cc: Christian Brauner
In-Reply-To: <20180927175857.3511-1-christian@brauner.io>

Various userspace programs (e.g. iproute2) have sent RTM_GETADDR
requests with struct ifinfomsg. This is wrong and should have been
struct ifaddrmsg all along as mandated by the manpages. However, dump
requests so far didn't parse the netlink message that was sent and
succeeded even when a wrong struct was passed along.

Currently, the message is parsed under the assumption that the correct
struct ifaddrmsg is sent down. If the parsing fails the kernel will
still fulfill the request to preserve backwards compatability but a
rate-limited message that there were leftover bytes after parsing the
message is recorded in dmesg. It has been argued that this is
unacceptable [1].

But various new features that got and will get added to RTM_GETADDR make
it necessary to definitely know what header was passed along.
This is currently not possible without resorting to (likely unreliable)
hacks such as introducing a nested attribute that ensures that
RTM_GETADDR which pass along properties such as IFA_TARGET_NETNSID
always exceed RTM_GETADDR requests that send down the wrong struct
ifinfomsg [2]. Basically, multiple approaches to solve this have been
shut down. Furthermore, the API expressed via RTM_GETADDR is apparently
frozen [3] so the wiggle room at this point seems very much zero.

The correct solution at this point seems to me to introduce a new
RTM_GETADDR2 request. This way we can parse the message and fail hard if
the struct is not struct ifaddrmsg and can safely extend it in the
future. Userspace tools that rely on the buggy RTM_GETADDR API will
still keep working without even having to see any log messages and new
userspace tools that want to make user of new features can make use of
the new RTM_GETADDR2 requests.

[1]: https://lists.openwall.net/netdev/2018/09/25/59
[2]: https://lists.openwall.net/netdev/2018/09/25/75
[3]: https://lists.openwall.net/netdev/2018/09/26/166

Signed-off-by: Christian Brauner <christian@brauner.io>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Benc <jbenc@redhat.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
---
 net/decnet/dn_dev.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/net/decnet/dn_dev.c b/net/decnet/dn_dev.c
index d0b3e69c6b39..8da6ca154c08 100644
--- a/net/decnet/dn_dev.c
+++ b/net/decnet/dn_dev.c
@@ -738,10 +738,12 @@ static void dn_ifaddr_notify(int event, struct dn_ifaddr *ifa)
 		rtnl_set_sk_err(&init_net, RTNLGRP_DECnet_IFADDR, err);
 }
 
-static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+static int dn_nl_dump_ifaddr_common(struct sk_buff *skb,
+				    struct netlink_callback *cb, int rtm_type)
 {
 	struct net *net = sock_net(skb->sk);
-	int idx, dn_idx = 0, skip_ndevs, skip_naddr;
+	int err, idx, dn_idx = 0, skip_ndevs, skip_naddr;
+	struct nlattr *tb[IFA_MAX+1];
 	struct net_device *dev;
 	struct dn_dev *dn_db;
 	struct dn_ifaddr *ifa;
@@ -749,6 +751,13 @@ static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	if (!net_eq(net, &init_net))
 		return 0;
 
+	if (rtm_type == RTM_GETADDR2) {
+		err = nlmsg_parse(cb->nlh, sizeof(struct ifaddrmsg), tb, IFA_MAX,
+				dn_ifa_policy, NULL);
+		if (err < 0)
+			return err;
+	}
+
 	skip_ndevs = cb->args[0];
 	skip_naddr = cb->args[1];
 
@@ -787,6 +796,16 @@ static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+static int dn_nl_dump_ifaddr(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	return dn_nl_dump_ifaddr_common(skb, cb, RTM_GETADDR);
+}
+
+static int dn_nl_dump_ifaddr2(struct sk_buff *skb, struct netlink_callback *cb)
+{
+	return dn_nl_dump_ifaddr_common(skb, cb, RTM_GETADDR2);
+}
+
 static int dn_dev_get_first(struct net_device *dev, __le16 *addr)
 {
 	struct dn_dev *dn_db;
@@ -1410,6 +1429,8 @@ void __init dn_dev_init(void)
 			     dn_nl_deladdr, NULL, 0);
 	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETADDR,
 			     NULL, dn_nl_dump_ifaddr, 0);
+	rtnl_register_module(THIS_MODULE, PF_DECnet, RTM_GETADDR2,
+			     NULL, dn_nl_dump_ifaddr2, 0);
 
 	proc_create_seq("decnet_dev", 0444, init_net.proc_net, &dn_dev_seq_ops);
 
-- 
2.17.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox