Re: setenv if value=NULL, what say standard? Bug?

[Rich Felker <dalias@libc.org>]

On Thu, Apr 23, 2015 at 12:24:46AM -0400, Jean-Marc Pigeon wrote:
> >> If this situation is indeed UB, there is 2 options for musl: 1)
> >> Swallow the problem nicely... as glibc and uclibc does. 2) Report
> >> an error.. EINVAL? (and document it in manual)
> >> 
> >> Crashing at "libc" level is not an option.
> > 
> > I can see how it might seem like that at first, but crashing is 
> > actually the best possible behavior. Options 1 and 2 cover up a
> I strongly disagree, crashing is not an option for a tools as
> musl/libc.
> 
> Think about this, you write an application working perfectly right,
> but 1 in 1000000 you reach something not trapped by low level and
> once in while the application (in production for month) just stop
> to work because "unexpected" within musl...

But that's not what's going on here, so it's a strawman. There is a
big difference in being robust against transient failures and ignoring
programming errors that are going to happen every single time the
program is run.

> (so someone will propose to set a cron to automatically restart this
> unreliable daemon, hmmm...)
> 
> Far better to return "trouble" status, then it is to the application
> to decide what must be done in context, as ignore, override, bypass,
> crash, etc.

If the application is sophisticated enough to check for errors in the
return value of setenv, it's also sophisticated enough to check that
the value it's passing is a valid pointer. As we've seen in the case
of hwclock, it does neither. And that was part of the point of the
text I linked: programs which invoke UB in ways like this, empirically
speaking, almost always completely return value checking.

> > potentially serious bug -- it's not clear what the application was 
> > trying to do, most likely nobody even thought about what they were 
> > trying to do, and even if they did have something in mind it's not 
> > reliable or portable. The glibc wiki has some text taken from text
> > I wrote on the topic (copied from a stack overflow answer I gave)
> > here:
> 
> As reported, the crashing application is hwclock, (util-linux-2.26),
> this a kind of code in the field for a very  very long time,

And it either crashes every time it's run (for a given configuration,
at least) or doesn't. If it does you know during early testing rather
than letting a bug slip through.

> so the
> library (glibc and old libc) used for linux over the years defined an
> expected behavior to this "UB".

No, that was merely a bug in glibc, not a feature.

> Something worry me in comments I have seen in the proposed URL,
> IMHO purpose of musl/glibc is not to "find bugs by crashing", its
> purpose is to be a code "clean, lean, reliable, predictable" (as said
> above, "Protect the hardware, report problem, lets the above layer
> application decide what to do in case of problem").

Part of protecting the system is avoiding any forward progress when
the application is known to be in an invalid/corrupt state due to UB.

> Crashing is not an option for code pertaining to musl/libc layer.

Crashing is inevitable on the vast majority of invalid programs.
setenv("TZ", (char *)0xdeadbeef, 1); will almost certainly crash, and
if it doesn't it will likely do something worse.

> (:-} why bother to return an error, just crash for all
> problems in open, close, write, etc. just bringing the crashing
> concept to the extreme :-}).

An error returned by open or write is not a consequence of any failure
by the programmer -- writing code with UB or otherwise. It's a
legitimate condition that can happen at runtime due to many possible
transient or permanent conditions like resource exhaustion,
non-existence of the file, permissions, etc.

You may notice that on many systems open fails with EFAULT when given
an invalid pointer rather than crashing. This is not particularly a
good thing. Consider code something like the following:

	int foo(const char *fn) {
		char buf[2];
		strcpy(buf, "hello world, and goodbye");
		int fd = open(fn ? fn : buf, O_RDONLY);
		return fd < 0 ? -1 : fd;
	}

Here fn is likely to be invalid at the time open is called due to the
buffer overflow in buf[]. When open ignores this and the program
continues running, it happily jumps to the clobbered return address.

Rich