From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yunsheng Lin Subject: Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path handling. Date: Tue, 19 Jun 2018 14:01:34 +0800 Message-ID: References: <20180619045802.24050-1-sudarsana.kalluru@cavium.com> <20180619045802.24050-2-sudarsana.kalluru@cavium.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Cc: , , To: Sudarsana Reddy Kalluru , Return-path: Received: from szxga05-in.huawei.com ([45.249.212.191]:8719 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751964AbeFSGCO (ORCPT ); Tue, 19 Jun 2018 02:02:14 -0400 In-Reply-To: <20180619045802.24050-2-sudarsana.kalluru@cavium.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 2018/6/19 12:58, Sudarsana Reddy Kalluru wrote: > Memory for packet buffers need to be freed in the error paths as there is > no consumer (e.g., upper layer) for such packets and that memory will never > get freed. > The issue was uncovered when port was attacked with flood of isatap > packets, these are multicast packets hence were directed at all the PFs. > For foce PF, this meant they were routed to the ll2 module which in turn > drops such packets. > > Fixes: 0a7fb11c ("qed: Add Light L2 support") > Signed-off-by: Sudarsana Reddy Kalluru > Signed-off-by: Ariel Elior > Signed-off-by: Michal Kalderon > --- > drivers/net/ethernet/qlogic/qed/qed_ll2.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > index c97ebd6..012973d 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c > @@ -201,8 +201,9 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) > > skb = build_skb(buffer->data, 0); > if (!skb) { > - rc = -ENOMEM; > - goto out_post; > + DP_INFO(cdev, "Failed to build SKB\n"); > + kfree(buffer->data); > + goto out_post1; > } > > data->u.placement_offset += NET_SKB_PAD; > @@ -224,8 +225,14 @@ void qed_ll2b_complete_rx_packet(void *cxt, struct qed_ll2_comp_rx_data *data) > cdev->ll2->cbs->rx_cb(cdev->ll2->cb_cookie, skb, > data->opaque_data_0, > data->opaque_data_1); > + } else { > + DP_VERBOSE(p_hwfn, (NETIF_MSG_RX_STATUS | NETIF_MSG_PKTDATA | > + QED_MSG_LL2 | QED_MSG_STORAGE), > + "Dropping the packet\n"); > + kfree(buffer->data); What about the memory used by skb itself? Does skb need to be freed by kfree_skb or something like that? > } > > +out_post1: > /* Update Buffer information and update FW producer */ > buffer->data = new_data; > buffer->phys_addr = new_phys_addr; >