Re: mktemp write failure

[Jim Meyering]

Eric Blake wrote:
> Eric Blake <ebb9 <at> byu.net> writes:
>> > mktemp -q >&-
>>
>> I'll be committing this to expose the bug, once I get freopen_safer
> implemented
>> in gnulib to fix the bug.
>
> This patch looks deceptively simple, since all the magic is in gnulib!

Yep, just the way it's supposed to be ;-)

> I also
> audited all other uses of freopen; changing stdin is unaffected, but changing
> any other stream requires this.  I didn't try to figure out if I could trigger
> inconsistencies with the other affected apps (cat, head, ptx, shuf, tac, tail,
> tee, tr, and uniq).  And we may want to use xfreopen in more places 
> (dircolors,
> du, mktemp, ptx, shuf, tsort, uniq), but I did not do that here.
>
>
>>From ccb6e85fa2cc21d132a116b5cf8e9f56bf747fe4 Mon Sep 17 00:00:00 2001
> From: Eric Blake <address@hidden>
> Date: Thu, 5 Nov 2009 12:19:45 -0700
> Subject: [PATCH 1/2] mktemp: fix bug with -q and closed stdout
>
> If stdin or stdout is closed, then freopen(,stderr) can violate
> the premise that STDERR_FILENO==fileno(stderr), which in turn
> breaks mktemp -q.
>
> * gnulib: Update, to pick up freopen_safer.
> * bootstrap.conf (gnulib_modules): Add freopen-safer.
> * tests/misc/close-stdout: Enhance test to catch bug.
> * src/mktemp.c (includes): Use stdio--.h.

I've tested both of these, after rebasing past my just-pushed gnulib update.
The above looks fine and all tests pass.

>>From 6b55bc3aeef3e08f1eeee6b85153f95969840891 Mon Sep 17 00:00:00 2001
> From: Eric Blake <address@hidden>
> Date: Thu, 5 Nov 2009 16:48:09 -0700
> Subject: [PATCH 2/2] build: consistently use freopen-safer
>
> cat, head, ptx, shuf, tac, tail, tee, tr, and uniq were affected

I'm pretty sure I've already audited each of those, and assured myself
that they were not vulnerable to this sort of trouble -- but that was back
when we added other *safer functions.  Thus, if they currently solve no
problem, I'm a little reluctant to impose the wrappers.  On the other,
I haven't measured the cost of going through the wrapper, and we've just
seen how the lack of the wrapper can cause an admittedly-obscure failure.

It is probably worth making this change regardless, just because
the issue is so subtle.

> * gl/modules/xfreopen (Depends-on): Add freopen-safer.
> * gl/lib/xfreopen.c (includes): Use stdio--.h.
> * src/ptx.c (includes): Likewise.
> * src/shuf.c (includes): Likewise.
> * src/uniq.c (includes): Likewise.

Have you considered automating (make syntax-check style) the task of
ensuring that stdio--.h is included at least whenever freopen is used?