The seq_file refactoring of hns3 debugfs removed the per-file entry state checks. Without them, debugfs show callbacks can access priv->ring, priv->tqp_vector, and ring->desc while these resources are being freed by reset, driver unload, or ethtool ring resize. Plain state-bit checks leave a TOCTOU window between check and use. Fix by adding a mutex (dbg_mutex) to struct hnae3_handle, shared by enet-layer and PF-layer debugfs callbacks via the common handle. Readers acquire the mutex and check device state at entry and per iteration -- the latter lets an in-flight reader exit promptly when reset begins mid-dump, keeping reset latency bounded. Writers clear INITED first to signal readers, then acquire the mutex before freeing resources. The dbg_uninit() call stays outside the mutex to avoid deadlock with debugfs_remove_recursive. Fixes: eced3d1c41db ("net: hns3: use seq_file for files in queue/ in debugfs") Closes: https://lore.kernel.org/all/a0853cd9-cab5-441d-b181-8ba97f2f58b0@huawei.com/ Signed-off-by: Jijie Shao --- drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 + .../ethernet/hisilicon/hns3/hns3_debugfs.c | 95 +++++++++++-------- .../net/ethernet/hisilicon/hns3/hns3_enet.c | 6 ++ .../ethernet/hisilicon/hns3/hns3_ethtool.c | 1 + .../hisilicon/hns3/hns3pf/hclge_debugfs.c | 14 +++ 5 files changed, 77 insertions(+), 40 deletions(-) diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h index 4286af9239b0..e94cc33da864 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h +++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h @@ -944,6 +944,7 @@ struct hnae3_handle { u8 netdev_flags; struct dentry *hnae3_dbgfs; + struct mutex dbg_mutex; /* protect debugfs against reset/unload */ /* Network interface message level enabled bits */ u32 msg_enable; diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c index 1347edac7699..7d913302f442 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_debugfs.c @@ -389,6 +389,12 @@ static const char * const dim_state_str[] = { "START", "IN_PROG", "APPLY" }; static const char * const dim_tune_stat_str[] = { "ON_TOP", "TIRED", "RIGHT", "LEFT" }; +static bool hns3_dbg_is_device_busy(struct hns3_nic_priv *priv) +{ + return !test_bit(HNS3_NIC_STATE_INITED, &priv->state) || + test_bit(HNS3_NIC_STATE_RESETTING, &priv->state); +} + static void hns3_get_coal_info(struct hns3_enet_tqp_vector *tqp_vector, struct seq_file *s, int i, bool is_tx) { @@ -434,7 +440,7 @@ static void hns3_get_coal_info(struct hns3_enet_tqp_vector *tqp_vector, } } -static void hns3_dump_coal_info(struct seq_file *s, bool is_tx) +static int hns3_dump_coal_info(struct seq_file *s, bool is_tx) { struct hnae3_handle *h = hnae3_seq_file_to_handle(s); struct hns3_enet_tqp_vector *tqp_vector; @@ -448,18 +454,32 @@ static void hns3_dump_coal_info(struct seq_file *s, bool is_tx) seq_puts(s, "HW_GL HW_QL\n"); for (i = 0; i < priv->vector_num; i++) { + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + tqp_vector = &priv->tqp_vector[i]; hns3_get_coal_info(tqp_vector, s, i, is_tx); } + + return 0; } static int hns3_dbg_coal_info(struct seq_file *s, void *data) { - hns3_dump_coal_info(s, true); - seq_puts(s, "\n"); - hns3_dump_coal_info(s, false); + struct hnae3_handle *h = hnae3_seq_file_to_handle(s); + struct hns3_nic_priv *priv = h->priv; + int ret; - return 0; + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + + ret = hns3_dump_coal_info(s, true); + if (ret) + return ret; + + seq_puts(s, "\n"); + return hns3_dump_coal_info(s, false); } static void hns3_dump_rx_queue_info(struct hns3_enet_ring *ring, @@ -504,22 +524,16 @@ static int hns3_dbg_rx_queue_info(struct seq_file *s, void *data) struct hns3_enet_ring *ring; u32 i; - if (!priv->ring) { - dev_err(&h->pdev->dev, "priv->ring is NULL\n"); - return -EFAULT; - } + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; seq_puts(s, "QUEUE_ID BD_NUM BD_LEN TAIL HEAD FBDNUM "); seq_puts(s, "PKTNUM COPYBREAK RING_EN RX_RING_EN BASE_ADDR\n"); for (i = 0; i < h->kinfo.num_tqps; i++) { - /* Each cycle needs to determine whether the instance is reset, - * to prevent reference to invalid memory. And need to ensure - * that the following code is executed within 100ms. - */ - if (!test_bit(HNS3_NIC_STATE_INITED, &priv->state) || - test_bit(HNS3_NIC_STATE_RESETTING, &priv->state)) - return -EPERM; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; ring = &priv->ring[(u32)(i + h->kinfo.num_tqps)]; hns3_dump_rx_queue_info(ring, s, i); @@ -569,22 +583,16 @@ static int hns3_dbg_tx_queue_info(struct seq_file *s, void *data) struct hns3_enet_ring *ring; u32 i; - if (!priv->ring) { - dev_err(&h->pdev->dev, "priv->ring is NULL\n"); - return -EFAULT; - } + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; seq_puts(s, "QUEUE_ID BD_NUM TC TAIL HEAD FBDNUM OFFSET "); seq_puts(s, "PKTNUM RING_EN TX_RING_EN BASE_ADDR\n"); for (i = 0; i < h->kinfo.num_tqps; i++) { - /* Each cycle needs to determine whether the instance is reset, - * to prevent reference to invalid memory. And need to ensure - * that the following code is executed within 100ms. - */ - if (!test_bit(HNS3_NIC_STATE_INITED, &priv->state) || - test_bit(HNS3_NIC_STATE_RESETTING, &priv->state)) - return -EPERM; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; ring = &priv->ring[i]; hns3_dump_tx_queue_info(ring, s, i); @@ -604,9 +612,14 @@ static int hns3_dbg_queue_map(struct seq_file *s, void *data) seq_puts(s, "local_queue_id global_queue_id vector_id\n"); + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + for (i = 0; i < h->kinfo.num_tqps; i++) { - if (!priv->ring || !priv->ring[i].tqp_vector) - continue; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + seq_printf(s, "%-16u%-17u%d\n", i, h->ae_algo->ops->get_global_queue_id(h, i), priv->ring[i].tqp_vector->vector_irq); @@ -661,8 +674,10 @@ static int hns3_dbg_rx_bd_info(struct seq_file *s, void *private) ring = &priv->ring[data->qid + data->handle->kinfo.num_tqps]; for (i = 0; i < ring->desc_num; i++) { - desc = &ring->desc[i]; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + desc = &ring->desc[i]; hns3_dump_rx_bd_info(priv, desc, s, i); } @@ -706,8 +721,10 @@ static int hns3_dbg_tx_bd_info(struct seq_file *s, void *private) ring = &priv->ring[data->qid]; for (i = 0; i < ring->desc_num; i++) { - desc = &ring->desc[i]; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; + desc = &ring->desc[i]; hns3_dump_tx_bd_info(desc, s, i); } @@ -796,10 +813,9 @@ static int hns3_dbg_page_pool_info(struct seq_file *s, void *data) struct hns3_enet_ring *ring; u32 i; - if (!priv->ring) { - dev_err(&h->pdev->dev, "priv->ring is NULL\n"); - return -EFAULT; - } + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; if (!priv->ring[h->kinfo.num_tqps].page_pool) { dev_err(&h->pdev->dev, "page pool is not initialized\n"); @@ -810,9 +826,8 @@ static int hns3_dbg_page_pool_info(struct seq_file *s, void *data) seq_puts(s, "POOL_SIZE(PAGE_NUM) ORDER NUMA_ID MAX_LEN\n"); for (i = 0; i < h->kinfo.num_tqps; i++) { - if (!test_bit(HNS3_NIC_STATE_INITED, &priv->state) || - test_bit(HNS3_NIC_STATE_RESETTING, &priv->state)) - return -EPERM; + if (hns3_dbg_is_device_busy(priv)) + return -EBUSY; ring = &priv->ring[(u32)(i + h->kinfo.num_tqps)]; hns3_dump_page_pool_info(ring, s, i); @@ -827,8 +842,8 @@ static int hns3_dbg_bd_info_show(struct seq_file *s, void *private) struct hnae3_handle *h = data->handle; struct hns3_nic_priv *priv = h->priv; - if (!test_bit(HNS3_NIC_STATE_INITED, &priv->state) || - test_bit(HNS3_NIC_STATE_RESETTING, &priv->state)) + guard(mutex)(&priv->ae_handle->dbg_mutex); + if (hns3_dbg_is_device_busy(priv)) return -EBUSY; if (data->cmd == HNAE3_DBG_CMD_TX_BD) diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c index 0c088feae03c..bae8b32ffc5b 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c @@ -5449,6 +5449,7 @@ static int hns3_client_init(struct hnae3_handle *handle) priv->min_tx_copybreak = 0; priv->min_tx_spare_buf_size = 0; set_bit(HNS3_NIC_STATE_DOWN, &priv->state); + mutex_init(&handle->dbg_mutex); handle->msg_enable = netif_msg_init(debug, DEFAULT_MSG_LEVEL); @@ -5562,6 +5563,7 @@ static int hns3_client_init(struct hnae3_handle *handle) priv->ring = NULL; out_get_ring_cfg: priv->ae_handle = NULL; + mutex_destroy(&handle->dbg_mutex); free_netdev(netdev); return ret; } @@ -5585,6 +5587,7 @@ static void hns3_client_uninit(struct hnae3_handle *handle, bool reset) hns3_free_rx_cpu_rmap(netdev); + mutex_lock(&handle->dbg_mutex); hns3_nic_uninit_irq(priv); hns3_clear_all_ring(handle, true); @@ -5596,9 +5599,11 @@ static void hns3_client_uninit(struct hnae3_handle *handle, bool reset) hns3_uninit_all_ring(priv); hns3_put_ring_config(priv); + mutex_unlock(&handle->dbg_mutex); out_netdev_free: hns3_dbg_uninit(handle); + mutex_destroy(&handle->dbg_mutex); free_netdev(netdev); } @@ -5875,6 +5880,7 @@ static int hns3_reset_notify_uninit_enet(struct hnae3_handle *handle) return 0; } + guard(mutex)(&handle->dbg_mutex); hns3_free_rx_cpu_rmap(netdev); hns3_nic_uninit_irq(priv); hns3_clear_all_ring(handle, true); diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c index 392653635bda..4e7a7e6b21ee 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c @@ -1258,6 +1258,7 @@ static int hns3_set_ringparam(struct net_device *ndev, if (if_running) ndev->netdev_ops->ndo_stop(ndev); + guard(mutex)(&h->dbg_mutex); hns3_change_all_ring_bd_num(priv, new_ringparam.tx_desc_num, new_ringparam.rx_desc_num); hns3_change_rx_buf_len(ndev, new_ringparam.rx_buf_len); diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c index 9a4e29bfa166..1e9d3e08b7de 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_debugfs.c @@ -15,6 +15,12 @@ #define hclge_seq_file_to_hdev(s) \ (((struct hnae3_ae_dev *)hnae3_seq_file_to_ae_dev(s))->priv) +static bool hclge_dbg_is_device_busy(struct hclge_dev *hdev) +{ + return test_bit(HCLGE_STATE_RST_HANDLING, &hdev->state) || + test_bit(HCLGE_STATE_RST_FAIL, &hdev->state); +} + static const char * const hclge_mac_state_str[] = { "TO_ADD", "TO_DEL", "ACTIVE" }; @@ -2621,6 +2627,10 @@ static int hclge_dbg_dump_umv_info(struct seq_file *s, void *data) struct hclge_vport *vport; u8 i; + guard(mutex)(&hdev->vport[0].nic.dbg_mutex); + if (hclge_dbg_is_device_busy(hdev)) + return -EBUSY; + seq_printf(s, "num_alloc_vport : %u\n", hdev->num_alloc_vport); seq_printf(s, "max_umv_size : %u\n", hdev->max_umv_size); seq_printf(s, "wanted_umv_size : %u\n", hdev->wanted_umv_size); @@ -2831,6 +2841,10 @@ static int hclge_dbg_dump_vlan_offload_config(struct hclge_dev *hdev, int ret; u8 i; + guard(mutex)(&hdev->vport[0].nic.dbg_mutex); + if (hclge_dbg_is_device_busy(hdev)) + return -EBUSY; + seq_puts(s, "FUNC_ID PVID ACCEPT_TAG1 ACCEPT_TAG2 ACCEPT_UNTAG1 "); seq_puts(s, "ACCEPT_UNTAG2 INSERT_TAG1 INSERT_TAG2 SHIFT_TAG "); seq_puts(s, "STRIP_TAG1 STRIP_TAG2 DROP_TAG1 DROP_TAG2 "); -- 2.43.0