DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ophir Munk <ophirmu@mellanox.com>
To: dev@dpdk.org, Pascal Mazon <pascal.mazon@6wind.com>
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Olga Shern <olgas@mellanox.com>,
	Ophir Munk <ophirmu@mellanox.com>
Subject: [dpdk-dev] [PATCH v2 1/2] net/tap: fix eBPF file descriptors leakage
Date: Tue, 30 Jan 2018 16:00:28 +0000	[thread overview]
Message-ID: <1517328029-17052-2-git-send-email-ophirmu@mellanox.com> (raw)
In-Reply-To: <1517328029-17052-1-git-send-email-ophirmu@mellanox.com>

When a user creates an RSS rule, the tap PMD dynamically allocates
a 'flow' data structure, and uploads BPF programs (represented by file
descriptors) to the kernel.
The kernel might reject the rule (due to filters overlap, for example)
in which case, flow memory should be freed and BPF file descriptors
should be closed.
In the corrupted code there were scenarios where BPF file descriptors
were not closed.
The fix is to add a new function - tap_flow_free(), which will make sure
to always close BPF file descriptors before freeing the flow allocated
memory.

Fixes: 036d721a8229 ("net/tap: implement RSS using eBPF")

Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
---
 drivers/net/tap/tap_flow.c | 57 ++++++++++++++++++++++++++++++----------------
 1 file changed, 38 insertions(+), 19 deletions(-)

diff --git a/drivers/net/tap/tap_flow.c b/drivers/net/tap/tap_flow.c
index 6aa53a7..533879d 100644
--- a/drivers/net/tap/tap_flow.c
+++ b/drivers/net/tap/tap_flow.c
@@ -212,6 +212,10 @@ tap_flow_create(struct rte_eth_dev *dev,
 		const struct rte_flow_action actions[],
 		struct rte_flow_error *error);
 
+static void
+tap_flow_free(struct pmd_internals *pmd,
+	struct rte_flow *flow);
+
 static int
 tap_flow_destroy(struct rte_eth_dev *dev,
 		 struct rte_flow *flow,
@@ -1311,6 +1315,38 @@ tap_flow_set_handle(struct rte_flow *flow)
 }
 
 /**
+ * Free the flow opened file descriptors and allocated memory
+ *
+ * @param[in] flow
+ *   Pointer to the flow to free
+ *
+ */
+static void
+tap_flow_free(struct pmd_internals *pmd, struct rte_flow *flow)
+{
+	int i;
+
+	if (!flow)
+		return;
+
+	if (pmd->rss_enabled) {
+		/* Close flow BPF file descriptors */
+		for (i = 0; i < SEC_MAX; i++)
+			if (flow->bpf_fd[i] != 0) {
+				close(flow->bpf_fd[i]);
+				flow->bpf_fd[i] = 0;
+			}
+
+		/* Release the map key for this RSS rule */
+		bpf_rss_key(KEY_CMD_RELEASE, &flow->key_idx);
+		flow->key_idx = 0;
+	}
+
+	/* Free flow allocated memory */
+	rte_free(flow);
+}
+
+/**
  * Create a flow.
  *
  * @see rte_flow_create()
@@ -1428,7 +1464,7 @@ tap_flow_create(struct rte_eth_dev *dev,
 	if (remote_flow)
 		rte_free(remote_flow);
 	if (flow)
-		rte_free(flow);
+		tap_flow_free(pmd, flow);
 	return NULL;
 }
 
@@ -1450,7 +1486,6 @@ tap_flow_destroy_pmd(struct pmd_internals *pmd,
 		     struct rte_flow_error *error)
 {
 	struct rte_flow *remote_flow = flow->remote_flow;
-	int i;
 	int ret = 0;
 
 	LIST_REMOVE(flow, next);
@@ -1476,22 +1511,6 @@ tap_flow_destroy_pmd(struct pmd_internals *pmd,
 			"couldn't receive kernel ack to our request");
 		goto end;
 	}
-	/* Close opened BPF file descriptors of this flow */
-	for (i = 0; i < SEC_MAX; i++)
-		if (flow->bpf_fd[i] != 0) {
-			close(flow->bpf_fd[i]);
-			flow->bpf_fd[i] = 0;
-		}
-
-	/* Release map key for this RSS rule */
-	ret = bpf_rss_key(KEY_CMD_RELEASE, &flow->key_idx);
-	if (ret < 0) {
-		rte_flow_error_set(
-			error, EINVAL, RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
-			"Failed to release BPF RSS key");
-
-		goto end;
-	}
 
 	if (remote_flow) {
 		remote_flow->msg.nh.nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
@@ -1520,7 +1539,7 @@ tap_flow_destroy_pmd(struct pmd_internals *pmd,
 end:
 	if (remote_flow)
 		rte_free(remote_flow);
-	rte_free(flow);
+	tap_flow_free(pmd, flow);
 	return ret;
 }
 
-- 
2.7.4

  reply	other threads:[~2018-01-30 16:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-29 11:18 [dpdk-dev] [PATCH v1] " Ophir Munk
2018-01-30 16:00 ` [dpdk-dev] [PATCH v2 0/2] fix eBPF TAP RSS leakage and map key handling Ophir Munk
2018-01-30 16:00   ` Ophir Munk [this message]
2018-01-30 16:00   ` [dpdk-dev] [PATCH v2 2/2] net/tap: fix eBPF RSS " Ophir Munk
2018-01-30 16:10   ` [dpdk-dev] [PATCH v2 0/2] fix eBPF TAP RSS leakage and " Pascal Mazon
2018-01-31 18:52     ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1517328029-17052-2-git-send-email-ophirmu@mellanox.com \
    --to=ophirmu@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=olgas@mellanox.com \
    --cc=pascal.mazon@6wind.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).