From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Hemminger <stephen@networkplumber.org>
Subject: Re: [PATCH v4 06/12] net/failsafe: add flexible device
 definition
Date: Wed, 31 May 2017 08:19:36 -0700
Message-ID: <20170531081936.2bbad988@xeon-e3>
References: <cover.1495634974.git.gaetan.rivet@6wind.com>
 <cover.1496065002.git.gaetan.rivet@6wind.com>
 <c740735828d7419881725b8441a4ddad58f7fbec.1496065002.git.gaetan.rivet@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: dev@dpdk.org
To: Gaetan Rivet <gaetan.rivet@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-pf0-f173.google.com (mail-pf0-f173.google.com
 [209.85.192.173]) by dpdk.org (Postfix) with ESMTP id E333E4CE4
 for <dev@dpdk.org>; Wed, 31 May 2017 17:19:38 +0200 (CEST)
Received: by mail-pf0-f173.google.com with SMTP id 9so12082607pfj.1
 for <dev@dpdk.org>; Wed, 31 May 2017 08:19:38 -0700 (PDT)
In-Reply-To: <c740735828d7419881725b8441a4ddad58f7fbec.1496065002.git.gaetan.rivet@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, 29 May 2017 15:42:18 +0200
Gaetan Rivet <gaetan.rivet@6wind.com> wrote:

>  
> +- **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.


> 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.