On Wed, May 31, 2017 at 08:19:36AM -0700, Stephen Hemminger wrote:

On Mon, 29 May 2017 15:42:18 +0200
Gaetan Rivet [off-list ref] wrote:

quoted

 
+- **exec(<shell command>)** parameter
+
+  This parameter allows the user to provide a command to the fail-safe PMD to
+  execute and define a sub-device.
+  It is done within a regular shell context.
+  The first line of its output is read by the fail-safe PMD and otherwise
+  interpreted as if passed by the regular **dev** parameter.
+  Any other line is discarded.
+  If the command fail or output an incorrect string, the sub-device is not
+  initialized.
+  All commas within the ``shell command`` are replaced by spaces before
+  executing the command. This helps using scripts to specify devices.
+

Exec from a DPDK application seems like possible security hole since most DPDK applications
have to run as root.

Users will run scripts or other programs that will launch fail-safe
instances. If a user launches a script over the fail-safe to configure
it or under it to detect devices, security seems at the same level?

quoted

static int
+fs_execute_cmd(struct sub_device *sdev, char *cmdline)
+{
+	FILE *fp;
+	/* store possible newline as well */
+	char output[DEVARGS_MAXLEN + 1];
+	size_t len;
+	int old_err;
+	int ret;
+
+	assert(cmdline != NULL || sdev->cmdline != NULL);
+	if (sdev->cmdline == NULL) {
+		char *new_str;
+		size_t i;
+
+		len = strlen(cmdline) + 1;
+		new_str = rte_realloc(sdev->cmdline, len,
+				RTE_CACHE_LINE_SIZE);
+		if (new_str == NULL) {
+			ERROR("Command line allocation failed");
+			return -ENOMEM;
+		}

Using rte_malloc for cmdline is way over optimizing. rte_malloc comes from huge page area
which is limited. The only reason to use it is if the memory needs to be shared by primary/slave.
Also rte_malloc has much less protection (memleak checkers, guards etc) compared to regular malloc.

I agree, it should be changed.

-- 
Gaëtan Rivet
6WIND