DPDK patches and discussions
 help / color / mirror / Atom feed
From: Shahaf Shuler <shahafs@mellanox.com>
To: Marcelo Ricardo Leitner <mleitner@redhat.com>
Cc: "Adrien Mazarguil" <adrien.mazarguil@6wind.com>,
	"Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2 2/4] net/mlx4: spawn rdma-core dependency plug-in
Date: Sun, 28 Jan 2018 11:46:59 +0000	[thread overview]
Message-ID: <VI1PR05MB314930D59E2FF6E6D6969F00C3E60@VI1PR05MB3149.eurprd05.prod.outlook.com> (raw)
In-Reply-To: <20180128111749.GJ3494@localhost.localdomain>

Sunday, January 28, 2018 1:18 PM, Marcelo Ricardo Leitner:
> Hi Shahaf,
> 
> On Sun, Jan 28, 2018 at 09:04:36AM +0000, Shahaf Shuler wrote:
> > Hi Marcelo,
> >
> > Saturday, January 27, 2018 5:03 PM, Marcelo Ricardo Leitner:
> > > On Fri, Jan 26, 2018 at 03:19:00PM +0100, Adrien Mazarguil wrote:
> > > ...
> > > > +static int
> > > > +mlx4_glue_init(void)
> > > > +{
> > > > +	char file[] = "/tmp/" MLX4_DRIVER_NAME "_XXXXXX";
> > > > +	int fd = mkstemp(file);
> > > ...
> > > > +	while (off != mlx4_glue_lib_size) {
> > > > +		ssize_t ret;
> > > > +
> > > > +		ret = write(fd, (const uint8_t *)mlx4_glue_lib + off,
> > > > +			    mlx4_glue_lib_size - off);
> > > > +		if (ret == -1) {
> > > > +			if (errno != EINTR) {
> > > > +				rte_errno = errno;
> > > > +				goto glue_error;
> > > > +			}
> > > > +			ret = 0;
> > > > +		}
> > > > +		off += ret;
> > > > +	}
> > > > +	close(fd);
> > > > +	fd = -1;
> > > > +	handle = dlopen(file, RTLD_LAZY);
> > > > +	unlink(file);
> > >
> > > This is a potential security issue. There are no guarantees that the
> > > file
> > > dlopen() will open is the file that was just written above. It could
> > > have been changed by something else in between.
> >
> > Can you further explain what are the potential risks you want to
> > protect from?
> 
> It is different in some aspects,
> 
> > I think this issue is not different from regular file protection under
> > Linux.
> 
> in regular files we can ensure the right selinux contexts and permissions are
> set, which is not the case here.
> 
> /usr could be even mounted as R/O and files just loaded from there, while
> with this approach you're allocating a new file on a temporary dir, which
> potentially is using extra RAM memory to store it and then load it.

So the issue is with the absolute path that contains the temporary file or with the concept of embedding the library inside the DPDK binary? 

> 
> >
> > If the DPDK process ran by root, then this approach is no less secure
> > than the previous version of the patches that dlopen the
> > /usr/lib/libibverbs.so and /usr/lib/libmlx5.so.  root can also change
> > them before the dlopen.
> 
> Maybe, and though that probably would leave traces in the system that that
> happened.
> 
> > In fact in terms of security, root user can intentionally damage the
> > system in many other ways.
> 
> And a SELinux (mis)configuration could grant (unexpected) write access to
> the file for some other, non-root, user.
> 
> >
> > If the DPDK process ran by regular user X, then the only users that
> > are allowed to modify the file created are user X and possibly root.
> > Other users will not have write permission to it.
> > if the same user change this temporary file, then it damages itself
> > only, as the DPDK process run by it will probably won't lunch.
> 
> Let's work this from the other PoV: why embed the library in dpdk binary?
> How is it any more stable than regular libraries?

The main reason is not to force the DPDK user to install the DPDK.
Using the approach suggest on this commit, DPDK user can just "make" DPDK and that is it. 
When the DPDK application starts, it spits a temporary file which happens to be usable with dlopen() and hard-linked with libibverbs and libmlx[45] and once loaded in memory, removes it from the file system.
 
dlopen the library itself will require the library to be in some well-known location. 

> 
> If it's just to hide it from rpm dependency generator, there is probably a
> cleaner way to do it. For example, packaging the lib in a sub-package, or
> maybe some more tricky rpm macro.

Do you see it as a preferred solution ? Having the glue lib included in rpms and install it per demand in case the binary execution failed?

> 
>   Marcelo

  reply	other threads:[~2018-01-28 11:47 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24 23:24 [dpdk-dev] [PATCH v1 0/4] net/mlx: make rdma-core optional at run-time Adrien Mazarguil
2018-01-24 23:25 ` [dpdk-dev] [PATCH v1 1/4] net/mlx4: move rdma-core calls to separate file Adrien Mazarguil
2018-01-24 23:58   ` Stephen Hemminger
2018-01-25 11:31     ` Adrien Mazarguil
2018-01-24 23:25 ` [dpdk-dev] [PATCH v1 2/4] net/mlx4: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-26 10:06   ` Thomas Monjalon
2018-01-24 23:25 ` [dpdk-dev] [PATCH v1 3/4] net/mlx5: move rdma-core calls to separate file Adrien Mazarguil
2018-01-24 23:25 ` [dpdk-dev] [PATCH v1 4/4] net/mlx5: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-26 14:18 ` [dpdk-dev] [PATCH v2 0/4] net/mlx: make rdma-core optional at run-time Adrien Mazarguil
2018-01-26 14:18   ` [dpdk-dev] [PATCH v2 1/4] net/mlx4: move rdma-core calls to separate file Adrien Mazarguil
2018-01-26 14:19   ` [dpdk-dev] [PATCH v2 2/4] net/mlx4: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-27 15:03     ` Marcelo Ricardo Leitner
2018-01-28  9:04       ` Shahaf Shuler
2018-01-28 11:17         ` Marcelo Ricardo Leitner
2018-01-28 11:46           ` Shahaf Shuler [this message]
2018-01-26 14:19   ` [dpdk-dev] [PATCH v2 3/4] net/mlx5: move rdma-core calls to separate file Adrien Mazarguil
2018-01-26 14:19   ` [dpdk-dev] [PATCH v2 4/4] net/mlx5: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-26 15:58   ` [dpdk-dev] [PATCH v2 0/4] net/mlx: make rdma-core optional at run-time Nélio Laranjeiro
2018-01-29 17:19   ` [dpdk-dev] [PATCH v3 " Adrien Mazarguil
2018-01-29 17:19     ` [dpdk-dev] [PATCH v3 1/4] net/mlx4: move rdma-core calls to separate file Adrien Mazarguil
2018-01-29 17:19     ` [dpdk-dev] [PATCH v3 2/4] net/mlx4: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-29 17:19     ` [dpdk-dev] [PATCH v3 3/4] net/mlx5: move rdma-core calls to separate file Adrien Mazarguil
2018-01-29 17:19     ` [dpdk-dev] [PATCH v3 4/4] net/mlx5: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-30 15:34     ` [dpdk-dev] [PATCH v4 0/4] net/mlx: make rdma-core optional at run-time Adrien Mazarguil
2018-01-30 15:34       ` [dpdk-dev] [PATCH v4 1/4] net/mlx4: move rdma-core calls to separate file Adrien Mazarguil
2018-01-30 15:34       ` [dpdk-dev] [PATCH v4 2/4] net/mlx4: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-30 17:54         ` Marcelo Ricardo Leitner
2018-01-30 18:32           ` Adrien Mazarguil
2018-01-30 15:34       ` [dpdk-dev] [PATCH v4 3/4] net/mlx5: move rdma-core calls to separate file Adrien Mazarguil
2018-01-30 15:34       ` [dpdk-dev] [PATCH v4 4/4] net/mlx5: spawn rdma-core dependency plug-in Adrien Mazarguil
2018-01-31 10:13       ` [dpdk-dev] [PATCH v4 0/4] net/mlx: make rdma-core optional at run-time Shahaf Shuler

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=VI1PR05MB314930D59E2FF6E6D6969F00C3E60@VI1PR05MB3149.eurprd05.prod.outlook.com \
    --to=shahafs@mellanox.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=mleitner@redhat.com \
    --cc=nelio.laranjeiro@6wind.com \
    /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).