From patchwork Thu Jan 26 01:02:05 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vadim Fedorenko X-Patchwork-Id: 13116478 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B6B0CC61D97 for ; Thu, 26 Jan 2023 01:02:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235954AbjAZBC6 (ORCPT ); Wed, 25 Jan 2023 20:02:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229662AbjAZBCa (ORCPT ); Wed, 25 Jan 2023 20:02:30 -0500 Received: from novek.ru (unknown [213.148.174.62]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 31B555CFF6 for ; Wed, 25 Jan 2023 17:02:29 -0800 (PST) Received: from nat1.ooonet.ru (gw.zelenaya.net [91.207.137.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by novek.ru (Postfix) with ESMTPSA id 2353150550A; Thu, 26 Jan 2023 03:57:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 novek.ru 2353150550A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=novek.ru; s=mail; t=1674694626; bh=A7ZV1g9EB+xsbzvbedI9XysNPerzOuuFHXWhsE+fTic=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZyRfhCMp1FAKom2js3FrV/WC/ZA1s35PlaWqGWlFLl+UcV6C81ERnKuxMY2sZhqEo ervc7giTm4wBIEcsZGLNoav0HUfnVOneQ03kRPSORsyaQvvW/IF+8J52f1rTorO4Ln mU3mk7W3OngnlnKy89PqTc0gvIVnGAcyl7ZCwpe4= From: Vadim Fedorenko To: Vadim Fedorenko , Aya Levin , Saeed Mahameed , Jakub Kicinski , Gal Pressman Cc: Vadim Fedorenko , netdev@vger.kernel.org Subject: [PATCH net v3 1/2] mlx5: fix skb leak while fifo resync and push Date: Thu, 26 Jan 2023 04:02:05 +0300 Message-Id: <20230126010206.13483-2-vfedorenko@novek.ru> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20230126010206.13483-1-vfedorenko@novek.ru> References: <20230126010206.13483-1-vfedorenko@novek.ru> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org From: Vadim Fedorenko During ptp resync operation SKBs were poped from the fifo but were never freed neither by napi_consume nor by dev_kfree_skb_any. Add call to napi_consume_skb to properly free SKBs. Another leak was happening because mlx5e_skb_fifo_has_room() had an error in the check. Comparing free running counters works well unless C promotes the types to something wider than the counter. In this case counters are u16 but the result of the substraction is promouted to int and it causes wrong result (negative value) of the check when producer have already overlapped but consumer haven't yet. Explicit cast to u16 fixes the issue. Fixes: 58a518948f60 ("net/mlx5e: Add resiliency for PTP TX port timestamp") Reviewed-by: Gal Pressman Signed-off-by: Vadim Fedorenko Reviewed-by: Tariq Toukan --- drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c | 6 ++++-- drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c index 8469e9c38670..b72de2b520ec 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c @@ -86,7 +86,8 @@ static bool mlx5e_ptp_ts_cqe_drop(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb return (ptpsq->ts_cqe_ctr_mask && (skb_cc != skb_id)); } -static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb_id) +static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, + u16 skb_id, int budget) { struct skb_shared_hwtstamps hwts = {}; struct sk_buff *skb; @@ -98,6 +99,7 @@ static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_ hwts.hwtstamp = mlx5e_skb_cb_get_hwts(skb)->cqe_hwtstamp; skb_tstamp_tx(skb, &hwts); ptpsq->cq_stats->resync_cqe++; + napi_consume_skb(skb, budget); skb_cc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_cc); } } @@ -119,7 +121,7 @@ static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq, } if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id)) - mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id); + mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id, budget); skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo); hwtstamp = mlx5e_cqe_ts_to_ns(sq->ptp_cyc2time, sq->clock, get_cqe_ts(cqe)); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h index 853f312cd757..15a5a57b47b8 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h @@ -81,7 +81,7 @@ void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq); static inline bool mlx5e_skb_fifo_has_room(struct mlx5e_skb_fifo *fifo) { - return (*fifo->pc - *fifo->cc) < fifo->mask; + return (u16)(*fifo->pc - *fifo->cc) < fifo->mask; } static inline bool From patchwork Thu Jan 26 01:02:06 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vadim Fedorenko X-Patchwork-Id: 13116479 X-Patchwork-Delegate: kuba@kernel.org Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1FE9CC27C76 for ; Thu, 26 Jan 2023 01:03:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236480AbjAZBC7 (ORCPT ); Wed, 25 Jan 2023 20:02:59 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236474AbjAZBCh (ORCPT ); Wed, 25 Jan 2023 20:02:37 -0500 Received: from novek.ru (unknown [213.148.174.62]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 702B65CFFC for ; Wed, 25 Jan 2023 17:02:35 -0800 (PST) Received: from nat1.ooonet.ru (gw.zelenaya.net [91.207.137.40]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by novek.ru (Postfix) with ESMTPSA id 249FA50550B; Thu, 26 Jan 2023 03:57:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 novek.ru 249FA50550B DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=novek.ru; s=mail; t=1674694633; bh=ZZ/ZmGvLqVhoaGKEaJ/0xM1UQavAEQko8za6zMQvHrs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Vg4FfR1bYtXYxr+XMRktm/xfkeFzrdHnCdZnSOvivHmdSP+Jp7znl5YvYZm9VbPf/ wW6BzWPd96lZZb8Ie74muzMyVrtouebs7fQ8WdW/GpzHGh+80HbAAzOAQ5viXFLhNX G0sx/+uq0qc7LjV/AyP8f9em9cIKIE7EY9WrBSGs= From: Vadim Fedorenko To: Vadim Fedorenko , Aya Levin , Saeed Mahameed , Jakub Kicinski , Gal Pressman Cc: Vadim Fedorenko , netdev@vger.kernel.org Subject: [PATCH net v3 2/2] mlx5: fix possible ptp queue fifo use-after-free Date: Thu, 26 Jan 2023 04:02:06 +0300 Message-Id: <20230126010206.13483-3-vfedorenko@novek.ru> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20230126010206.13483-1-vfedorenko@novek.ru> References: <20230126010206.13483-1-vfedorenko@novek.ru> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org X-Patchwork-Delegate: kuba@kernel.org From: Vadim Fedorenko Fifo pointers were not checked during push and pop operations and this could potentially lead to use-after-free or skb leak under heavy PTP traffic. Also there were OOO cqe spotted which lead to drain of the queue and use-after-free because of lack of fifo pointers check. Special check is added to avoid resync operation if SKB could not exist in the fifo because of OOO cqe (skb_id must be between consumer and producer index). Fixes: 58a518948f60 ("net/mlx5e: Add resiliency for PTP TX port timestamp") Signed-off-by: Vadim Fedorenko --- .../net/ethernet/mellanox/mlx5/core/en/ptp.c | 23 ++++++++++++++----- .../net/ethernet/mellanox/mlx5/core/en/txrx.h | 7 +++++- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c index b72de2b520ec..4ac7483dcbcc 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c @@ -86,7 +86,7 @@ static bool mlx5e_ptp_ts_cqe_drop(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb return (ptpsq->ts_cqe_ctr_mask && (skb_cc != skb_id)); } -static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, +static bool mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_cc, u16 skb_id, int budget) { struct skb_shared_hwtstamps hwts = {}; @@ -94,14 +94,23 @@ static void mlx5e_ptp_skb_fifo_ts_cqe_resync(struct mlx5e_ptpsq *ptpsq, u16 skb_ ptpsq->cq_stats->resync_event++; - while (skb_cc != skb_id) { - skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo); + if (skb_cc > skb_id || PTP_WQE_CTR2IDX(ptpsq->skb_fifo_pc) < skb_id) + pr_err_ratelimited("mlx5e: out-of-order ptp cqe\n"); + return false; + } + + while (skb_cc != skb_id && (skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo))) { hwts.hwtstamp = mlx5e_skb_cb_get_hwts(skb)->cqe_hwtstamp; skb_tstamp_tx(skb, &hwts); ptpsq->cq_stats->resync_cqe++; napi_consume_skb(skb, budget); skb_cc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_cc); } + + if (!skb) + return false; + + return true; } static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq, @@ -111,7 +120,7 @@ static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq, u16 skb_id = PTP_WQE_CTR2IDX(be16_to_cpu(cqe->wqe_counter)); u16 skb_cc = PTP_WQE_CTR2IDX(ptpsq->skb_fifo_cc); struct mlx5e_txqsq *sq = &ptpsq->txqsq; - struct sk_buff *skb; + struct sk_buff *skb = NULL; ktime_t hwtstamp; if (unlikely(MLX5E_RX_ERR_CQE(cqe))) { @@ -120,8 +129,10 @@ static void mlx5e_ptp_handle_ts_cqe(struct mlx5e_ptpsq *ptpsq, goto out; } - if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id)) - mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id, budget); + if (mlx5e_ptp_ts_cqe_drop(ptpsq, skb_cc, skb_id) && + !mlx5e_ptp_skb_fifo_ts_cqe_resync(ptpsq, skb_cc, skb_id, budget)) { + goto out; + } skb = mlx5e_skb_fifo_pop(&ptpsq->skb_fifo); hwtstamp = mlx5e_cqe_ts_to_ns(sq->ptp_cyc2time, sq->clock, get_cqe_ts(cqe)); diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h index 15a5a57b47b8..6e559b856afb 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/txrx.h @@ -289,14 +289,19 @@ struct sk_buff **mlx5e_skb_fifo_get(struct mlx5e_skb_fifo *fifo, u16 i) static inline void mlx5e_skb_fifo_push(struct mlx5e_skb_fifo *fifo, struct sk_buff *skb) { - struct sk_buff **skb_item = mlx5e_skb_fifo_get(fifo, (*fifo->pc)++); + struct sk_buff **skb_item; + WARN_ONCE(mlx5e_skb_fifo_has_room(fifo), "ptp fifo overflow"); + skb_item = mlx5e_skb_fifo_get(fifo, (*fifo->pc)++); *skb_item = skb; } static inline struct sk_buff *mlx5e_skb_fifo_pop(struct mlx5e_skb_fifo *fifo) { + if (*fifo->pc == *fifo->cc) + return NULL; + return *mlx5e_skb_fifo_get(fifo, (*fifo->cc)++); }