patches for DPDK stable branches
 help / color / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Jin Yu <jin.yu@intel.com>
Cc: Maxime Coquelin <maxime.coquelin@redhat.com>,
	Tiwei Bie <tiwei.bie@intel.com>,
	Zhihong Wang <zhihong.wang@intel.com>,
	dev@dpdk.org, stable@dpdk.org
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] vhost: fix insecure temporary file
Date: Tue, 26 Nov 2019 10:00:21 +0000
Message-ID: <20191126100021.GC1622@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20191126151900.70915-1-jin.yu@intel.com>

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 <jin.yu@intel.com>
> ---
>  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

  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-26 15:19 [dpdk-stable] " Jin Yu
2019-11-26 10:00 ` Bruce Richardson [this message]
2019-11-27  2:19   ` [dpdk-stable] [dpdk-dev] " Yu, Jin
2019-11-27  9:32     ` Bruce Richardson

Reply instructions:

You may reply publically 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=20191126100021.GC1622@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jin.yu@intel.com \
    --cc=maxime.coquelin@redhat.com \
    --cc=stable@dpdk.org \
    --cc=tiwei.bie@intel.com \
    --cc=zhihong.wang@intel.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

patches for DPDK stable branches

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/stable/0 stable/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 stable stable/ http://inbox.dpdk.org/stable \
		stable@dpdk.org
	public-inbox-index stable


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.stable


AGPL code for this site: git clone https://public-inbox.org/ public-inbox