[MINC-development] patches for tmpnam

[Robert VINCENT]

Hi Vicka,

I tend to agree that the annoying linker message is worth fixing, even if
the result isn't perfect.  But I don't want to create a problem with file
descriptor leakage, either.

What I've done elsewhere is create a private function named something like
"miget_temp_filename(char *)" which calls mkstemp() if that function is
available, otherwise it calls tempnam().

This partially fixes the race condition, since the central issue with
tempnam() is that it doesn't actually create a file, whereas mkstemp()
creates a zero-length file with the permissions 600.  I think it is
therefore much harder for an attacker to exploit mkstemp() than tempnam().

My scheme requires adding a probe for "mkstemp()" in the configure script,
so it's a lot of effort to get rid of one warning.  But such is life...

I'm happy to make these changes if there is general agreement.  Are there
other suggestions?

Thanks,

	-bert

On Thu, 6 Mar 2003, Vicka Corey wrote:

> On Wed, Mar 05, 2003 at 10:07:06PM -0500, Steve ROBBINS wrote:
> > On Mon, Mar 03, 2003 at 05:06:51PM -0500, Vicka Corey wrote:
> > > Hi -- here are the changes I've put in to get rid of gcc warnings
> > > from calls to tmpnam().
> >
> > The changes you propose will cause file descriptor leakage which
> > is not acceptable, especially not in a library.
> >
> > It's true that you can get around that by closing the file
> > descriptor returned by mkstemp(), but that really misses the
> > point of the warnings.  The linker is warning you of a race
> > condition.  If the tmpnam() calls are going to be replaced,
> > the race ought to be fixed.  There's no point in doing it just
> > to get rid of a link warning.
>
> Since the warnings were causing various of our developers to modify
> private copies of your code and use different versions of it and of
> the compilers, actually getting rid of the warnings is *very*
> important to us.  It's hard to debug your code when you have to
> wade through a lot of irrelevant output, which is why I'm trying
> to request changes in the MNI code in the first place.
>
> Fixing race condition and leaks is worth doing too, of course, but
> please don't minimize the need for code to compile cleanly.
>
> Did the tmpnam() and tempnam() calls in the code clean up after themselves?
> If so, the same closes should still work; if not it is at least no worse.
>
> > P.S.  GCC 3 no longer emits this particular warning, anyway.
>
> See above about developers using different versions of compilers.
>
> > P.P.S. The man page for mkstemp() on my system says "Don't use this function,
> > use tmpfile(3) instead.  It is better defined and more portable."
>
> Under gcc 3.1, the tmpfile() man page says,
> BUGS
>      These interfaces are provided for System V and ANSI compatibility only.
>      The mkstemp(3) interface is strongly preferred.
> [...]
>
> --vicka
> _______________________________________________
> MINC-development mailing list
> MINC-development@bic.mni.mcgill.ca
> http://www.bic.mni.mcgill.ca/mailman/listinfo/minc-development
>