-----Original Message-----
From: Yunsheng Lin [mailto:linyunsh...@huawei.com]
Sent: 19 June 2018 11:32
To: Kalluru, Sudarsana <sudarsana.kall...@cavium.com>; da...@davemloft.net
Cc: netdev@vger.kernel.org; Elior, Ariel <ariel.el...@cavium.com>; Kalderon,
Michal <michal.kalde...@cavium.com>
Subject: Re: [PATCH net 1/3] qed: Fix possible memory leak in Rx error path
handling.
External Email
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 <sudarsana.kall...@cavium.com>
> Signed-off-by: Ariel Elior <ariel.el...@cavium.com>
> Signed-off-by: Michal Kalderon <michal.kalde...@cavium.com>
> ---
> 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?
[Sudarsana] Thanks for reviewing the changes. qed_ll2_alloc_buffer() allocates
this memory. The allocated buffer (i.e., buffer->data) holds complete memory
for 'skb + data' as required by build_skb() implementation. Hence freeing of
(buffer->data) would suffice here.
> }
>
> +out_post1:
> /* Update Buffer information and update FW producer */
> buffer->data = new_data;
> buffer->phys_addr = new_phys_addr;
>
