On Wed, May 31, 2017 at 08:19:36AM -0700, Stephen Hemminger wrote:
> On Mon, 29 May 2017 15:42:18 +0200
> Gaetan Rivet <gaetan.ri...@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.
>
>
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
