[FFmpeg-devel] [PATCH] wrap mkstemp with umask

[Reimar Döffinger]

Hello,
On Sun, Jul 20, 2008 at 01:52:07PM -0700, Erik Hovland wrote:
> On Thu, Jul 17, 2008 at 09:39:33PM +0200, Reimar D?ffinger wrote:
> > On Thu, Jul 17, 2008 at 12:28:45PM -0700, Erik Hovland wrote:
> > > It is possible that the current umask of the process creating the file
> > > with mkstemp is such that the temp file could be writable by others then
> > > the user. When mkstemp is used it can be wrapped with umask() calls to
> > > make absolutely sure that the temp file cannot be tampered with.
> > 
> > That makes it completely thread-unsafe.
> 
> Is the only caller using threads? It is likely that the code is not
> thread-safe to begin with because it has two calls to mkstemp where it
> changes the template. Whoever uses this call now should really hold a
> lock (even w/out the umask calls).

Why should the two mkstemps be a big problem here? The problem with
umask is that it affects the global program state, so the behaviour of
(e.g. GUI-)threads that have nothing at all to do with FFmpeg will be
changed. Also, given the place where this code is, proper locking
basically means a global lock that is used for the whole encoding step,
for GUI programs that would mean the GUI updating and processing can not
be done in parallel - which is a really big step backwards.

> But I am all for removing the code altogether. It seems that
> libav{codec,format} is not really in the business of managing files.

Well, the best solution would still be a patch to make xvid skip the
file reading and directly accept in-memory stats. Should not really be
difficult (though maybe a bit ugly), just needs to be done.

Greetings,
Reimar D?ffinger