From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E7E4FA04C3; Tue, 26 Nov 2019 11:00:30 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 17CCE2C60; Tue, 26 Nov 2019 11:00:29 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id D20F528EE; Tue, 26 Nov 2019 11:00:27 +0100 (CET) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Nov 2019 02:00:26 -0800 X-IronPort-AV: E=Sophos;i="5.69,245,1571727600"; d="scan'208";a="202668734" Received: from bricha3-mobl.ger.corp.intel.com ([10.251.85.102]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 26 Nov 2019 02:00:24 -0800 Date: Tue, 26 Nov 2019 10:00:21 +0000 From: Bruce Richardson To: Jin Yu Cc: Maxime Coquelin , Tiwei Bie , Zhihong Wang , dev@dpdk.org, stable@dpdk.org Message-ID: <20191126100021.GC1622@bricha3-MOBL.ger.corp.intel.com> References: <20191126151900.70915-1-jin.yu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191126151900.70915-1-jin.yu@intel.com> User-Agent: Mutt/1.12.1 (2019-06-15) Subject: Re: [dpdk-dev] [PATCH] vhost: fix insecure temporary file 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Tue, Nov 26, 2019 at 11:19:00PM +0800, Jin Yu wrote: > When using mkstemp(), remember to safely set the umask > before to restrict the resulting temporary file > permissions to only the owner. > > Coverity issue: 350367 > Fixes: d87f1a1cb7b6 ("vhost: support inflight info sharing") > Cc: stable@dpdk.org > > Signed-off-by: Jin Yu > --- > lib/librte_vhost/vhost_user.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 0cfb8b792..1a68e23e3 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -1342,6 +1342,7 @@ inflight_mem_alloc(const char *name, size_t size, int *fd) > RTE_SET_USED(name); > #endif > if (mfd == -1) { > + mode_t mask = umask(0600); > mfd = mkstemp(fname); Setting the umask is unnecessary, as if you read the man page for mkstemp: "The file is created with permissions 0600, that is, read plus write for owner only." I am aware that coverity flags this as a potential issue, but if you follow the link from the coverity issue to CWE-377 on cwe.mitre.org, you can find the following at the end of the "Notes" section: "Finally, mkstemp() is a reasonably safe way create temporary files. It will attempt to create and open a unique file based on a filename template provided by the user combined with a series of randomly generated characters. If it is unable to create such a file, it will fail and return -1. On modern systems the file is opened using mode 0600, which means the file will be secure from tampering unless the user explicitly changes its access permissions. However, mkstemp() still suffers from the use of predictable file names and can leave an application vulnerable to denial of service attacks if an attacker causes mkstemp() to fail by predicting and pre-creating the filenames to be used." So it seems that for creating temporary files, mkstemp() is probably the best function we can use. Therefore, I recommend not trying to patch this issue and just mark the issue as "ignore" in coverity. Regards, /Bruce