Re: [Qemu-trivial] [Qemu-devel] [PATCH] slirp: use less predictable directory name in /tmp for smb config (CVE-2015-4037)

[Markus Armbruster]

Michael Tokarev <address@hidden> writes:

> 01.06.2015 11:01, Markus Armbruster wrote:
> []
>>> -    snprintf(s->smb_dir, sizeof(s->smb_dir), "/tmp/qemu-smb.%ld-%d",
>>> -             (long)getpid(), instance++);
>>> -    if (mkdir(s->smb_dir, 0700) < 0) {
>>> +    strcpy(s->smb_dir, "/tmp/qemu-smb.XXXXXX");
>>> +    if (!mkdtemp(s->smb_dir)) {
>>>          error_report("could not create samba server dir '%s'", s->smb_dir);
>>>          return -1;
>>>      }
>> 
>> Uh, I have to suspend my R-by.  This clobbers s->smb_dir[] when
>> mkdtemp() fails.  Is that safe?  Explain why, and get my R-by back.
>
> At least this is not different from the original behavour.

Fair enough, R-by unsuspended.

> The only place where this variable is used is in the slirp_smb_cleanup()
> function, which, if smb_dir[0] != 0, does rm -rf on it.
> I think it will be better to add s->smb_dir[0] = 0 here before return.

Makes sense.

> On the other hand this isn't really that important given when we can
> expect mkdtemp() to fail.

Not sure I got this part.  Not sure I need to :)