From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46478) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z1n7i-00080J-3D for qemu-devel@nongnu.org; Sun, 07 Jun 2015 22:49:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z1n7e-0000pg-2D for qemu-devel@nongnu.org; Sun, 07 Jun 2015 22:49:14 -0400 Date: Thu, 4 Jun 2015 06:38:53 +0200 From: Miroslav Rezanina Message-ID: <20150604043853.GA2485@lws.brq.redhat.com> References: <1433223995-26725-1-git-send-email-mjt@msgid.tls.msk.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1433223995-26725-1-git-send-email-mjt@msgid.tls.msk.ru> Subject: Re: [Qemu-devel] [PATCH v2] slirp: use less predictable directory name in /tmp for smb config (CVE-2015-4037) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: qemu-trivial@nongnu.org, Jan Kiszka , qemu-devel@nongnu.org, Markus Armbruster On Tue, Jun 02, 2015 at 08:46:35AM +0300, Michael Tokarev wrote: > In this version I used mkdtemp(3) which is: > > _BSD_SOURCE > || /* Since glibc 2.10: */ > (_POSIX_C_SOURCE >= 200809L || _XOPEN_SOURCE >= 700) > > (POSIX.1-2008), so should be available on systems we care about. > > While at it, reset the resulting directory name within smb structure > on error so cleanup function wont try to remove directory which we > failed to create. > > Signed-off-by: Michael Tokarev > --- > v2: > Add resetting of the dirname on failure so that cleanup function > does not try to remove directory which we failed to create. > > Use snprintf() as was in the original code, not strcpy(): while > in this very case it does not matter at all since both strings > are of known size, some people dislike strcpy() in principle. > > net/slirp.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/net/slirp.c b/net/slirp.c > index 0e15cf6..3533837 100644 > --- a/net/slirp.c > +++ b/net/slirp.c > @@ -481,7 +481,6 @@ static void slirp_smb_cleanup(SlirpState *s) > static int slirp_smb(SlirpState* s, const char *exported_dir, > struct in_addr vserver_addr) > { > - static int instance; > char smb_conf[128]; > char smb_cmdline[128]; > struct passwd *passwd; > @@ -505,10 +504,10 @@ static int slirp_smb(SlirpState* s, const char *exported_dir, > return -1; > } > > - snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d", > - (long)getpid(), instance++); > - if (mkdir(s->smb_dir, 0700) < 0) { > + snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.XXXXXX"); > + if (!mkdtemp(s->smb_dir)) { > error_report("could not create samba server dir '%s'", s->smb_dir); > + s->smb_dir[0] = 0; > return -1; > } > snprintf(smb_conf, sizeof(smb_conf), "%s/%s", s->smb_dir, "smb.conf"); > -- > 2.1.4 > > Reviewed-by: Miroslav Rezanina