Hi Theo,
Theo de Raadt wrote on Wed, Jul 26, 2017 at 11:27:03PM -0600:
> Ted Unangst wrote:
>> returning -1 to indicate error, ignoring the possibility of short
>> output, seems like the option that results in less damage. as an
>> application author, it's the only behavior i can reasonably code
>> against.
> I don't believe that. It may be possible to come to a conclusion like
> that, if review of a large body of code found at least a few checks
> for -1 / ENOMEM, or just -1 on it's own. If no old instances are found,
> is that a case of people not being reasonable application authors?
>
> No, I think it is just a gap -- really nothing new.
Admittedly, the vast majority of [f]printf(3) uses ignore return
values even when floating point numbers are printed, so the ecosystem
looks worse than for snprintf(3) and asprintf(3). Then again, many
uses are non-critical debugging or diagnostic output, so maybe
there are not that many serious bugs, but who knows.
But you do find careful checks in unusually well-written software
(abbreviated):
/usr/src/usr.sbin/ntpd/ntpd.c
int writefreq(double d) {
r = fprintf(freqfp, "%.3f\n", d * 1e6); /* scale to ppm */
if (r < 0 || fflush(freqfp) != 0) {
So, our current libc can trick ntpd(8) into silently writing a
corrupt driftfile. Do you say that check must be changed into
if (r <= 0 || fflush(freqfp) != 0) {
I'm sure even fewer code authors would expect that to be needed.
Kristaps wrote such a check even in mandoc (admittedly, the handling
of the error is wrong, it still results in a broken PDF file, but
at least the check is there):
/usr/src/usr.bin/mandoc/term_ps.c
static void ps_printf(struct termp *p, const char *fmt, ...) {
va_start(ap, fmt);
len = vprintf(fmt, ap);
va_end(ap);
p->ps->pdfbytes += len < 0 ? 0 : (size_t)len;
I say returning -1 is the safe thing to do. Most software of average
quality will behave erratically either way. Returning a positive
value on malloc failure specifically punishes software that is
written with above-average care.
Yours,
Ingo
