Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 23 Apr 2015 08:29:45 -0400
From: Jean-Marc Pigeon <jmp@...e.ca>
To: musl@...ts.openwall.com
Subject: Re: setenv if value=NULL, what say standard? Bug?

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/23/2015 01:08 AM, Raphael Cohn wrote:
> Fail fast, fail early is always the right thing to do. I've seen
> far too much code in my career that tries to 'carry on' after
> encountering an unexpected condition. It's always the wrong thing
> to do, even for nuclear power plants. Once you're in UB land, the
> rest of your reasoning about your code is invalid. The best you can
> do is gracefully shutdown and capture as much diagnostic info as is
> feasible. Expecting musl to 'cover up' should be seen the same as
> in the real world - cover ups just make the problem worse.
this is UB, man setenv(name, NULL, override)  is/was not defining if
it was valid or not.

Code must be resilient.

Either report an error and have the documentation updated accordingly
(get rid of the UB, have all old running code taking care of the new
standard) or code the 'best' decision implementing code (done by glibc
or uclibc so fare).



> 
> Unexpected is not the same as expected but unpleasant to deal with
> eg failures to open(). And so hence the arguments about checked
> and unchecked exceptions and so on.
> 
> Oh and auto restarting daemons is almost always wrong. For exactly
> the same reasoning - and frequently leads to data corruption.

Fully agreed with you and this was exactly my point, but reality say
otherwise, in the field sometime it is seen as the only option :-{{{.



> 
> On 23 Apr 2015 05:25, "Jean-Marc Pigeon" <jmp@...e.ca 
> <mailto:jmp@...e.ca>> wrote:
> 
> On 04/22/2015 10:15 PM, Rich Felker wrote:
>> On Wed, Apr 22, 2015 at 09:26:57PM -0400, Jean-Marc Pigeon
>> wrote:
>>>> I think the only safe conclusion is that the application is 
>>>> incorrect and should ensure that setenv() is never called
>>>> with a NULL value.
>>>> 
>>> Checked glibc, My understanding, it set something as "name="
>>> in the environment, so the variable is present but value is
>>> "empty"i (top application to decide what to do). uclibc does
>>> something similar (as far I can tell looking at source code)..
>>> 
>>> 
>>> The application is not careful enough, but not incorrect as 
>>> such.
> 
>> It's definitely incorrect. It's doing something that invokes 
>> undefined behavior.
> 
>>> Note: we may have tons of applications with the same problem.
>>> if we keep musl setenv like that, musl will be seen as quite 
>>> unreliable.
> 
>> No, actually glibc is fixing this bug (maybe they already did). 
>> See the thread beginning here:
> 
>> https://sourceware.org/ml/libc-alpha/2015-03/threads.html#00449
> 
>> My understanding is that glibc is planning to do, or already
>> does in the latest version, exactly what musl is doing.
> 
>>> 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... (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.
> 
> A sensible policy in case of UB would be for such low level code
> to swallow the problem, (protect the hardware and keep the program 
> running as much as possible).
> 
>> 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, so the library (glibc and old libc) used for linux
> over the years defined an expected behavior to this "UB". As other
> occurrence of this could be present too in other 
> program/application, crashing would make a bench of applications to
> be running hazard (you can have all kind situation with env
> variables, near unpredictable).
> 
> 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").
> 
> Crashing is not an option for code pertaining to musl/libc layer.
> 
> (:-} why bother to return an error, just crash for all problems in
> open, close, write, etc. just bringing the crashing concept to the
> extreme :-}).
> 
> 
> 
> 
> 
> https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers
>
> 
>> Specifically it covers why returning an error is not a good
>> idea.
> My experience (for a long time now) about writing complex daemon 
> running for months/year, it is not that straightforward (may be for
> a simple application it is)
> 
> 
>> Rich
> 
> 
> 
> 

- -- 

A bientôt
===========================================================
Jean-Marc Pigeon                        E-Mail: jmp@...e.ca
SAFE Inc.                             Phone: (514) 493-4280
  Clement, 'a kiss solution' to get rid of SPAM (at last)
     Clement' Home base <"http://www.clement.safe.ca">
===========================================================
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJVOOW5AAoJEAtgwJ4bBQU52jEQAJXCLoQyl0s4E9uQaAOsmA8x
TEuDRsRI0wn90QaW5GdwbpUDwzPqp9d33FXkxpnQljoGX71sH17AmXfe9L7h32pG
RQnlpRQy5zdxXYx1tioUvLfwfMW+PsqG5SwyzmNOKvoBMS4sC7FHrwJtYh9YZYSB
hO7C6tdS8b7hJoRwzW1/4KPJRak7Z3dTD2tZiMk+4VWtzs19HQ6nwMMjAHBIw4l3
EbcLVjdjCcFVtbWC8Wr/nJQcDoAenUXrj5qfxxazPjb1AJGa91k4eGbSS7FDFiqA
JT4tdpgYL4/+RkwzbjJhUJHXBisYJ29KBpq/YV3Yj/BOlSjT0z2yfu/dUlU37x/m
dDP535oty1jR4l3t73yumyiXP41c8tdwK22QAaaK+4Mh+1SdkA+Eh9rsZsl7cij0
UrvLTrc4sNjD+zIvEhkdyhCZ/4SVrXpVCpxHOXO2IIB/1CDTu2a63OaNnpctrrK9
VHsGTgAZgmwGkszXUhK8cIsw4FQNog1U2CwcwJBJhk0UGkwQOG4HXtECvqD9YjhQ
jIY2tFzEeG7ObwMT0AjK0KKd/aVKoCyCfNLjO3Pucww54/8aTQaWZ/mOj4fwsP01
YTrbAs2L4VDRtnHQru4THYIjKMK8P0zU991XqPhTeMGhJuP5x13cHzAoUhsw3HVh
q1Ki9yFz/20JUbK/KqRt
=VNMJ
-----END PGP SIGNATURE-----


Download attachment "smime.p7s" of type "application/pkcs7-signature" (4242 bytes)

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.