Re: [Qemu-devel] [PATCH 1/3] qtest: Enable creation of multiple qemu instances

[Blue Swirl]

On Mon, Dec 17, 2012 at 5:13 PM, Jason Baron <address@hidden> wrote:
> On Sat, Dec 15, 2012 at 09:20:13AM +0000, Blue Swirl wrote:
>> On Sat, Dec 15, 2012 at 9:14 AM, Paolo Bonzini <address@hidden> wrote:
>> >> > +#define QTEST_FILE_TEMP "/tmp/qtest-%d.sock"
>> >> > +#define QTEST_QMP_FILE_TEMP "/tmp/qtest-%d.qmp"
>> >> > +#define QTEST_PID_FILE_TEMP "/tmp/qtest-%d.pid"
>> >>
>> >> These filenames are too predictable from security point of view,
>> >
>> > This need not be secure as long as the file is created with 0600
>> > permissions.  In fact, inspecting the pid file from the shell can
>> > be useful.
>>
>> Permissions do not help at all because the attacker could for example
>> target overwriting of a critical file.
>>
>> >
>> > However, using mkstemp() on a prefix that includes the parent pid
>> > can indeed be the best of both worlds.
>>
>> Yes.
>>
>> >
>> > Paolo
>>
>
> Yes, but mkstemp() creates the file, and bind() returns EADDRINUSE, if the 
> file
> already exists.
>
> Using mktemp() in this case, with bind() should be ok, since bind() checks if
> the file exists and then creates it, if not, all within the bind() system call
> (so its atomic).

mktemp() manual page warns against using it, tempnam() looks like a
better choice.

>
> Thanks,
>
> -Jason