From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 5FD121B28B for ; Sun, 28 Jan 2018 12:17:52 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 3890D461C5; Sun, 28 Jan 2018 11:17:51 +0000 (UTC) Received: from dhcpe234.fit.vutbr.cz (ovpn-204-31.brq.redhat.com [10.40.204.31]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EDEC35EDE4; Sun, 28 Jan 2018 11:17:50 +0000 (UTC) Received: by dhcpe234.fit.vutbr.cz (Postfix, from userid 1000) id 249A818091C; Sun, 28 Jan 2018 09:17:49 -0200 (-02) Date: Sun, 28 Jan 2018 09:17:49 -0200 From: Marcelo Ricardo Leitner To: Shahaf Shuler Cc: Adrien Mazarguil , =?iso-8859-1?Q?N=E9lio?= Laranjeiro , "dev@dpdk.org" Message-ID: <20180128111749.GJ3494@localhost.localdomain> References: <20180124223625.1928-1-adrien.mazarguil@6wind.com> <20180126141215.30395-1-adrien.mazarguil@6wind.com> <20180126141215.30395-3-adrien.mazarguil@6wind.com> <20180127150306.GH3494@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.1 (2017-09-22) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Sun, 28 Jan 2018 11:17:51 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH v2 2/4] net/mlx4: spawn rdma-core dependency plug-in X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 Jan 2018 11:17:52 -0000 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. > > 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? 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. Marcelo