From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bl2nam02on0129.outbound.protection.outlook.com ([104.47.38.129]:33184 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1730411AbeIGFRT (ORCPT ); Fri, 7 Sep 2018 01:17:19 -0400 From: Sasha Levin To: "stable@vger.kernel.org" , "linux-kernel@vger.kernel.org" CC: Jakub Kicinski , "David S . Miller" , Sasha Levin Subject: [PATCH AUTOSEL 4.14 53/67] nfp: avoid buffer leak when FW communication fails Date: Fri, 7 Sep 2018 00:38:00 +0000 Message-ID: <20180907003716.57737-53-alexander.levin@microsoft.com> References: <20180907003716.57737-1-alexander.levin@microsoft.com> In-Reply-To: <20180907003716.57737-1-alexander.levin@microsoft.com> Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org List-ID: From: Jakub Kicinski [ Upstream commit 07300f774fec9519663a597987a4083225588be4 ] After device is stopped we reset the rings by moving all free buffers to positions [0, cnt - 2], and clear the position cnt - 1 in the ring. We then proceed to clear the read/write pointers. This means that if we try to reset the ring again the code will assume that the next to fill buffer is at position 0 and swap it with cnt - 1. Since we previously cleared position cnt - 1 it will lead to leaking the first buffer and leaving ring in a bad state. This scenario can only happen if FW communication fails, in which case the ring will never be used again, so the fact it's in a bad state will not be noticed. Buffer leak is the only problem. Don't try to move buffers in the ring if the read/write pointers indicate the ring was never used or have already been reset. nfp_net_clear_config_and_disable() is now fully idempotent. Found by code inspection, FW communication failures are very rare, and reconfiguring a live device is not common either, so it's unlikely anyone has ever noticed the leak. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller Signed-off-by: Sasha Levin --- drivers/net/ethernet/netronome/nfp/nfp_net_common.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c b/drivers/= net/ethernet/netronome/nfp/nfp_net_common.c index 8d53a593fb27..aba1113b2392 100644 --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c @@ -1071,7 +1071,7 @@ static bool nfp_net_xdp_complete(struct nfp_net_tx_ri= ng *tx_ring) * @dp: NFP Net data path struct * @tx_ring: TX ring structure * - * Assumes that the device is stopped + * Assumes that the device is stopped, must be idempotent. */ static void nfp_net_tx_ring_reset(struct nfp_net_dp *dp, struct nfp_net_tx_ring *tx_ri= ng) @@ -1273,13 +1273,18 @@ static void nfp_net_rx_give_one(const struct nfp_ne= t_dp *dp, * nfp_net_rx_ring_reset() - Reflect in SW state of freelist after disable * @rx_ring: RX ring structure * - * Warning: Do *not* call if ring buffers were never put on the FW freelis= t - * (i.e. device was not enabled)! + * Assumes that the device is stopped, must be idempotent. */ static void nfp_net_rx_ring_reset(struct nfp_net_rx_ring *rx_ring) { unsigned int wr_idx, last_idx; =20 + /* wr_p =3D=3D rd_p means ring was never fed FL bufs. RX rings are alway= s + * kept at cnt - 1 FL bufs. + */ + if (rx_ring->wr_p =3D=3D 0 && rx_ring->rd_p =3D=3D 0) + return; + /* Move the empty entry to the end of the list */ wr_idx =3D D_IDX(rx_ring, rx_ring->wr_p); last_idx =3D rx_ring->cnt - 1; @@ -2489,6 +2494,8 @@ static void nfp_net_vec_clear_ring_data(struct nfp_ne= t *nn, unsigned int idx) /** * nfp_net_clear_config_and_disable() - Clear control BAR and disable NFP * @nn: NFP Net device to reconfigure + * + * Warning: must be fully idempotent. */ static void nfp_net_clear_config_and_disable(struct nfp_net *nn) { --=20 2.17.1