From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet Subject: Re: [PATCH v4 06/12] net/failsafe: add flexible device definition Date: Thu, 1 Jun 2017 16:24:21 +0200 Message-ID: <20170601142421.GD18840@bidouze.vm.6wind.com> References: <20170531081936.2bbad988@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org To: Stephen Hemminger Return-path: Received: from mail-wm0-f47.google.com (mail-wm0-f47.google.com [74.125.82.47]) by dpdk.org (Postfix) with ESMTP id 652BF7CBF for ; Thu, 1 Jun 2017 16:24:28 +0200 (CEST) Received: by mail-wm0-f47.google.com with SMTP id b84so160838700wmh.0 for ; Thu, 01 Jun 2017 07:24:28 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20170531081936.2bbad988@xeon-e3> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, May 31, 2017 at 08:19:36AM -0700, Stephen Hemminger wrote: > On Mon, 29 May 2017 15:42:18 +0200 > Gaetan Rivet wrote: > > > > > +- **exec()** 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? > > 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