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

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

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

Hello,


hwclock (util-linux-2.26) is crashing, this is caused by
setenv.

#0  mktime_tz (tm=..., universal=1, valid_p=0x7fffffffea4c,
systime_p=0x7fffffffea30) at sys-utils/hwclock.c:411
#1  0x00000000004025f6 in read_hardware_clock (universal=1,
valid_p=0x7fffffffea4c, systime_p=0x7fffffffea30)
    at sys-utils/hwclock.c:476
#2  0x0000000000403bae in manipulate_clock (show=1, adjust=0,
noadjfile=0, set=0, set_time=0, hctosys=0,
    systohc=0, systz=0, startup_time=..., utc=0, local_opt=0,
update=0, testing=0, predict=0, get=0)
    at sys-utils/hwclock.c:1336
#3  0x0000000000404b37 in main (argc=0, argv=0x7fffffffebe8) at
sys-utils/hwclock.c:1965

src:
hwclock.c:411 setenv("TZ", getenv("TZUTC"), TRUE);

as TZUTC is undefined (NULL),
setenv is crashing

setenv.c:30 l2 = strlen(value);

fixing the hwclock code is a possibility,
but we could have other same-kind situation
far less obvious in other application, making
all sort of code to crash "randomly".

Tried to see what was saying standard,
about  (const char *value) to be NULL
when calling
setenv(const char *name, const char *value, int overwrite);
found nothing.

My guess, glibc code is 'blindly" setting the NULL (as "")
value to the variable.

Is the standard saying otherwise, or do we have a
a real bug in setenv??




- -- 

A bientôt
===========================================================
Jean-Marc Pigeon                        E-Mail: jmp@safe.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/

iQIcBAEBAgAGBQJVODfxAAoJEAtgwJ4bBQU5Aj4QAIvcx2qmpFwJEYfF4CzL2c/M
mAYTToDCGmyQcsLg0ZNTn/jfIFDuJ4wlY5piNgY/uFOg+3MwddEs1QhZhjZt9jeW
50cDffYYF18EMBuCC6D7gF1kIsMUmGUavvJbQCq3fTfhhIHTQoScDm9fM8EcU0jl
wywmos8XYgGRgIxOGKCkqmqG6luAjvZQcjsubG+CPb5aGRJndljCwZAIL1fXwjiL
QBjD1Ww2KTXlUM0bduIbYKPTVO/cTM3y+RqiN5BtQG7G1ipMBbDiyHlY5RqV3mZ7
07vcXwwC5G+DLOtGe7ZXQLIt7SgGR24rsXMT0O+kyRqkCXJ0KJBf78ezC05hQwvr
H+V7kSaDHMcfBc85cCf5I1iOdST+2dx5S7VZ+O7/0DPudCda6DcezWgVCxr6zfbw
Ow1GH/QjcmLGf73lXez1ccSKNeP5FOJxgFoKKDaO5iYxG7iEig2LKLEvK4uMATLv
VZfPOIdL7RmEe0Nt+aXe3X07DrJkPxbEq6nVzG8kJE2Jm1YAipcsDYcmrngaM3Hf
9tZ/RXqCNquOjitUTQrBesn0nIGa83woPo/FwGPWdH3nuQWjtvOMZ9z1sNmy3AXx
85eQxZDWGyYQ0VP/iQAttvWLV8L+nY46tJs2curFMsHwcucHHprRke4VTQxy+FD6
f3wAVbTKx3VlNwcsgJ7y
=lWjh
-----END PGP SIGNATURE-----


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Laurent Bercot]

On 23/04/2015 02:08, Jean-Marc Pigeon wrote:
> My guess, glibc code is 'blindly" setting the NULL (as "")
> value to the variable.
>
> Is the standard saying otherwise, or do we have a
> a real bug in setenv??

  The standard at
  http://pubs.opengroup.org/onlinepubs/9699919799/functions/setenv.html
  says...

  ... exactly nothing about the possibility of envval being NULL.
This is, in the strictest sense, UB. :)

  Actually, it says "The environment variable shall be set to the value
to which envval points." So, arguably, envval should point to something,
and since NULL does not, it is forbidden. Another valid interpretation
could be that envvar is set to the value to which envval points, i.e.
no value at all, so it is unset; but it doesn't fit the spirit of
setenv() to unset variables. The glibc interpretation, if it does what
you think it does, is wrong in any case: the empty string is a very
different thing from no value at all.

  I think the only safe conclusion is that the application is incorrect
and should ensure that setenv() is never called with a NULL value.

-- 
  Laurent

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 3255 bytes --]

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

On 04/22/2015 08:35 PM, Laurent Bercot wrote:
> On 23/04/2015 02:08, Jean-Marc Pigeon wrote:
>> My guess, glibc code is 'blindly" setting the NULL (as "") value
>> to the variable.
>> 
>> Is the standard saying otherwise, or do we have a a real bug in
>> setenv??
> 
> The standard at 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/setenv.html
>
> 
says...
> 
> ... exactly nothing about the possibility of envval being NULL. 
> This is, in the strictest sense, UB. :)
> 
> Actually, it says "The environment variable shall be set to the
> value to which envval points." So, arguably, envval should point to
> something, and since NULL does not, it is forbidden. Another valid
> interpretation could be that envvar is set to the value to which
> envval points, i.e. no value at all, so it is unset; but it doesn't
> fit the spirit of setenv() to unset variables. The glibc
> interpretation, if it does what you think it does, is wrong in any
> case: the empty string is a very different thing from no value at
> all.
> 
> 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.
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.

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.



- -- 

A bientôt
===========================================================
Jean-Marc Pigeon                        E-Mail: jmp@safe.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/

iQIcBAEBAgAGBQJVOEphAAoJEAtgwJ4bBQU5ryMP/1p/gbq19bmhvWB8EWhuXQvC
tunPUHRmeM20J1RaEP437ISVUqcbzoiJAxiyrOBAZYQsM02I4+OPWlncYPfWfGgF
oqZ13GvvQn8rPGdBTDmSC5r2MSMI9na/FplWi638IjlKyehejJ/zzwMBszI7WA+H
6/wIgeTK1nh3Gegr6qSQmMcUBSH6GH5JLYqa1JAoN/YCi82JWyqAnnS/lZrfFH/2
HrQ2l4yFv8Ed9ofPJQ2Rz1h9if81zldqCyy2LXx8BZXEfaYdOOXpjoX9y71A671s
Cq/zj+nSWocZdLX5Bf1jFdXyvm5YXjQAsX5EMQ4nXdIFmcDZX5Z48+gU/PBgpRM2
+H7fy4aXW0eYSd7LlCVi/0Xf+p/YDm6JJCCNQCKArIUc14M/9Gl0+5y6quvwL8YX
tt8ffuj0F/OLwfQybd0pBLATFWsknsXdCA5ADPkDQ+YJIgx5UuHjediFRDo6ooCM
o0e5/yqHX3tWu9troJaxGNK0Ntm2eu0ZHSm1wuLE+lBKX5aqxCoKktTFMtD2Xk+Z
ZsBego9M79tCvWd76EalckwzjogJLDlMb5cKToJKxeZ6UyX56mIL8+SWPfZNTzy5
6P590SsBmdMC8mMv36sx54SpmIBQvaU21pp0a9LnJRba5JUmXIyWZR/1Srme2l6k
jA6srmhKxZH6mQ94qik/
=W3QH
-----END PGP SIGNATURE-----


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Rich Felker]

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
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:

https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers

Specifically it covers why returning an error is not a good idea.

Rich

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 5365 bytes --]

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

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@safe.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/

iQIcBAEBAgAGBQJVOHQOAAoJEAtgwJ4bBQU5bCUP/jxt74112bqFat0+CvFpqNG8
sCEYEIpRcKIB7wWRLwQXQTQ0M82oUzo6bmpYtgorcwJWh5NIo/dFm2FvDW+/uiTs
dtutt45jJN7tsq4BhE/z/jyD44vrVqYZ+gJXf3MFPWeTFx4Kd7aCq3dCDvvoT9lk
Mp9KiiSY+WXfwE/a8CAiw6D+Ma2/iN4zTD1fUekXcZDgu0iMsieeF5uhZNT/L+62
U8K2VdOoR1H731v3GzKFUiesPTiPNbASg0MDfKyYe5kCZMkijbHIx7fGb/VID6dv
01u3bNA8yTfj51MplGv3ddgAzYcOMBMrQ6IXBK4hQPm4wuGp3ELr6IiTepUfkc3y
WYlNHCFqpgMthj2+nsMllwK+uZcbW36+QlYQ4FCuAMaqOjpA+Yr6qmoiOl5HFSMA
FoqarDkUeH3jYWYBFq8aUFQdD8Esyj3mKXAc/Bw45vWGkBykTyJXMYGVh4idukFQ
dkiYTM5rOB3H55dkjB9EnojAX+w0+VG+0H5xtb8fybtQYEr7SbsRO5fhTbmTr/1P
Q6ojm8fhkoRT0JXC+94htj5rT/87QerVIu06YcozqOsqCKBm7HBkokkMTMtWofJ4
sznm9/OtNqbYYTSdDQ7AVn1x0K8ycz5Aw302NOWd5NuSEPal+GuWP2mewhydlXAj
2+W/ox353kVOSWf0t2Ql
=R+I2
-----END PGP SIGNATURE-----


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Raphael Cohn]

[-- Attachment #1: Type: text/plain, Size: 6488 bytes --]

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.

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.
On 23 Apr 2015 05:25, "Jean-Marc Pigeon" <jmp@safe.ca> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> 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@safe.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/
>
> iQIcBAEBAgAGBQJVOHQOAAoJEAtgwJ4bBQU5bCUP/jxt74112bqFat0+CvFpqNG8
> sCEYEIpRcKIB7wWRLwQXQTQ0M82oUzo6bmpYtgorcwJWh5NIo/dFm2FvDW+/uiTs
> dtutt45jJN7tsq4BhE/z/jyD44vrVqYZ+gJXf3MFPWeTFx4Kd7aCq3dCDvvoT9lk
> Mp9KiiSY+WXfwE/a8CAiw6D+Ma2/iN4zTD1fUekXcZDgu0iMsieeF5uhZNT/L+62
> U8K2VdOoR1H731v3GzKFUiesPTiPNbASg0MDfKyYe5kCZMkijbHIx7fGb/VID6dv
> 01u3bNA8yTfj51MplGv3ddgAzYcOMBMrQ6IXBK4hQPm4wuGp3ELr6IiTepUfkc3y
> WYlNHCFqpgMthj2+nsMllwK+uZcbW36+QlYQ4FCuAMaqOjpA+Yr6qmoiOl5HFSMA
> FoqarDkUeH3jYWYBFq8aUFQdD8Esyj3mKXAc/Bw45vWGkBykTyJXMYGVh4idukFQ
> dkiYTM5rOB3H55dkjB9EnojAX+w0+VG+0H5xtb8fybtQYEr7SbsRO5fhTbmTr/1P
> Q6ojm8fhkoRT0JXC+94htj5rT/87QerVIu06YcozqOsqCKBm7HBkokkMTMtWofJ4
> sznm9/OtNqbYYTSdDQ7AVn1x0K8ycz5Aw302NOWd5NuSEPal+GuWP2mewhydlXAj
> 2+W/ox353kVOSWf0t2Ql
> =R+I2
> -----END PGP SIGNATURE-----
>
>

[-- Attachment #2: Type: text/html, Size: 7836 bytes --]

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

[Isaac Dunham]

On Thu, Apr 23, 2015 at 02:35:15AM +0200, Laurent Bercot wrote:
> On 23/04/2015 02:08, Jean-Marc Pigeon wrote:
> >My guess, glibc code is 'blindly" setting the NULL (as "")
> >value to the variable.
> >
> >Is the standard saying otherwise, or do we have a
> >a real bug in setenv??
> 
>  The standard at
>  http://pubs.opengroup.org/onlinepubs/9699919799/functions/setenv.html
>  says...
> 
>  ... exactly nothing about the possibility of envval being NULL.
> This is, in the strictest sense, UB. :)
> 
>  Actually, it says "The environment variable shall be set to the value
> to which envval points." So, arguably, envval should point to something,
> and since NULL does not, it is forbidden. Another valid interpretation
> could be that envvar is set to the value to which envval points, i.e.
> no value at all, so it is unset; but it doesn't fit the spirit of
> setenv() to unset variables. The glibc interpretation, if it does what
> you think it does, is wrong in any case: the empty string is a very
> different thing from no value at all.
> 
>  I think the only safe conclusion is that the application is incorrect
> and should ensure that setenv() is never called with a NULL value.

I happen to have just reread
http://www.tedunangst.com/flak/post/zero-size-objects
which has this little bit of information:

  The C standard has this to say in the section on “Use of library
  functions”.

  If an argument to a function has an invalid value (such as a value
  outside the domain of the function, or a pointer outside the address
  space of the program, or a null pointer, [...]) [...], the behavior
  is undefined.

  The section on “String function conventions” clarifies further.

  Where an argument declared as size_t n specifies the length of the
  array for a function, n can have the value zero on a call to that
  function. Unless explicitly stated otherwise in the description of
  a particular function in this subclause, pointer arguments on such
  a call shall still have valid values.

In other words, passing a NULL pointer is undefined behavior unless
spelled out to the contrary, even if a good implementation would have
no reason to follow it.

Now, setenv() is part of POSIX rather than ISO C, and thus has its
own rules, but ISO C is the foundation.

As to the question of what's the "right" thing to do, consider these
two function calls:
    setenv("OTHERVAL", getenv("SOMEVAL"),  1);
    strcmp(getenv("OTHERVAL"), getenv("OTHERVAL"));

It should be obvious that the second is incorrect.
But it's easy to arrive there if the former is accepted.

So I've sent a patch for this to the util-linux list.


Thanks,
Isaac Dunham

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

[Jens Gustedt]

[-- Attachment #1: Type: text/plain, Size: 3793 bytes --]

Hello

Am Mittwoch, den 22.04.2015, 22:15 -0400 schrieb Rich Felker:
> 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.

You probably mean it *has* undefined behavior. UB is nothing that can
be invoked.

Yes, it actually has two bugs in the case that was the starting point
of this thread. It has to calls to libc functions where it doesn't
check the return values.

> > 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
> 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:
> 
> https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers
> 
> Specifically it covers why returning an error is not a good idea.

I see your point, but I would go a bit more moderately and more
pragmatically with it.

First of all UB is what it is, a specific standard (here POSIX)
doesn't impose any form of behavior. So an implementation may extend
the behavior as it pleases. (Otherwise these standards have a saying
"it is unspecified whether ... or not ...")

Now, failing early is certainly a good property when we can expect
just that; any application that uses the call in that way *will* in
fact fail early. This is particularly important in code that otherwise
will have a performance penalty for doing checks.

Another acceptable strategy, IMPOV, is to forward errors where this is
easy to do and the check doesn't impose an unacceptable penalty. The
application then can handle the error (or not).

setenv is certainly borderline. Code for which it is performance
critical is almost certainly broken in many ways, and on the other
hand failures in the way we have seen here can be rare and late.

So I would have a small preference for being nice: do nothing to the
environment and return an error.

Just my 2 ¢

Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

[Laurent Bercot]

On 23/04/2015 06:24, Jean-Marc Pigeon wrote:
> 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...

  And why do you think the problem exists in the first place ?
Because other libcs were defensive and failed to fail early, so the
bug was never discovered until now. Your application is not working
perfectly right - it is buggy, and it *should* fail. musl is giving
developers a gift that other libcs do not: it helps them debug.


> (so someone will propose to set a cron to automatically restart this
> unreliable daemon, hmmm...)

  You want to be defensive, well, yeah, this is the place to be
defensive. Until the bug is found and fixed, at least the daemon is
kind of providing service.

  Raphael says this behaviour is wrong for the same reason that
silently failing is wrong, but I disagree. First, restarting crashing
daemons is not silent at all, a crash is always a loud warning and
can hardly be ignored; and second, restarting a process is not
continuing it. A process can always be restarted from a clean state
and work in a predictable way until it trips the bug again, whereas
silently ignoring UB makes the process unpredictable for the rest of
its lifetime.


> 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.

  What "trouble" status do you return when a function dereferences a
NULL pointer ? This is exactly what's happening here. Passing NULL
to setenv is as incorrect as dereferencing NULL, and should result
in the same behaviour.


> 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).

  The language you want is Javascript, not C.


> 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".

  And this is why musl is so much better. If glibc and uclibc devs
hadn't been so complacent, the bug wouldn't have lived for so long.


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

  It definitely is. You don't want your program to crash ? Don't
invoke UB.
  If you want to be "safe", you can ignore SIGSEGV at the start of
all your applications - it will be the exact same thing as what you
are asking. Your daemons will live longer, I guarantee it.


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

  Straw man. You know as well as we do the difference between a
programming error and a run-time error.


> 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)

  And mine is that it is. We're evens, now please let's stop bringing
up anecdotal evidence.

-- 
  Laurent

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

[Raphael Cohn]

[-- Attachment #1: Type: text/plain, Size: 4441 bytes --]

On 23 April 2015 at 10:23, Laurent Bercot <ska-dietlibc@skarnet.org> wrote:

> On 23/04/2015 06:24, Jean-Marc Pigeon wrote:
>
>> 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...
>>
>
>  And why do you think the problem exists in the first place ?
> Because other libcs were defensive and failed to fail early, so the
> bug was never discovered until now. Your application is not working
> perfectly right - it is buggy, and it *should* fail. musl is giving
> developers a gift that other libcs do not: it helps them debug.
>
>
>  (so someone will propose to set a cron to automatically restart this
>> unreliable daemon, hmmm...)
>>
>
>  You want to be defensive, well, yeah, this is the place to be
> defensive. Until the bug is found and fixed, at least the daemon is
> kind of providing service.
>
>  Raphael says this behaviour is wrong for the same reason that
> silently failing is wrong, but I disagree. First, restarting crashing
> daemons is not silent at all, a crash is always a loud warning and
> can hardly be ignored; and second, restarting a process is not
> continuing it. A process can always be restarted from a clean state
> and work in a predictable way until it trips the bug again, whereas
> silently ignoring UB makes the process unpredictable for the rest of
> its lifetime.


Yes and no. Crashes are loud and noisy, and should immediately trigger
alerts, but without intimate knowledge of the application and the cause of
the fault, auto-restarting is risky. In my operational experience, it's
usually been a hack employed by incompetent sysadmins (no names, no pack
drill, but one large government dept comes to mind). If you have knowledge
of your daemon processes, then you could if:-
- you know they are idempotent or do not have persistent state (eg DNS
caches)
- they're essential system services (definitions might vary, but I'd have
ssh for geographically remote boxes here)
 That said, stuff that has complex state really shouldn't be restarted
without *investigation* - message brokers, relational database titans,
cluster HA set ups, etc. The worst outage of my career was a terracotta
cluster that had suffered from a split brain. Restarting it naively caused
it to _delete_ the only remaining good state. Is this your 'clean state'
caveat above?

>
>
>  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.
>>
>
>  What "trouble" status do you return when a function dereferences a
> NULL pointer ? This is exactly what's happening here. Passing NULL
> to setenv is as incorrect as dereferencing NULL, and should result
> in the same behaviour.
>
>
>  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).
>>
>
>  The language you want is Javascript, not C.
>
>
>  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".
>>
>
>  And this is why musl is so much better. If glibc and uclibc devs
> hadn't been so complacent, the bug wouldn't have lived for so long.
>
>
>  Crashing is not an option for code pertaining to musl/libc layer.
>>
>
>  It definitely is. You don't want your program to crash ? Don't
> invoke UB.
>  If you want to be "safe", you can ignore SIGSEGV at the start of
> all your applications - it will be the exact same thing as what you
> are asking. Your daemons will live longer, I guarantee it.
>
>
>  (:-} why bother to return an error, just crash for all
>> problems in open, close, write, etc. just bringing the crashing
>> concept to the extreme :-}).
>>
>
>  Straw man. You know as well as we do the difference between a
> programming error and a run-time error.
>
>
>  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)
>>
>
>  And mine is that it is. We're evens, now please let's stop bringing
> up anecdotal evidence.
>
> --
>  Laurent
>
>

[-- Attachment #2: Type: text/html, Size: 6449 bytes --]

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

[Rich Felker]

On Thu, Apr 23, 2015 at 10:05:01AM +0200, Jens Gustedt wrote:
> Hello
> 
> Am Mittwoch, den 22.04.2015, 22:15 -0400 schrieb Rich Felker:
> > 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.
> 
> You probably mean it *has* undefined behavior. UB is nothing that can
> be invoked.

This is rather standard wording, and is meant to emphasize that all
the rules of undefined behavior come into play (i.e. are invoked).

> Yes, it actually has two bugs in the case that was the starting point
> of this thread. It has to calls to libc functions where it doesn't
> check the return values.

And this failure to check the return value was exactly my point in the
text I cited about why it's not a good idea to return an error on UB:
programs sufficiently buggy to be doing things like this in practice
almost never check return values.

> > > 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
> > 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:
> > 
> > https://sourceware.org/glibc/wiki/Style_and_Conventions#Invalid_pointers
> > 
> > Specifically it covers why returning an error is not a good idea.
> 
> I see your point, but I would go a bit more moderately and more
> pragmatically with it.
> 
> First of all UB is what it is, a specific standard (here POSIX)
> doesn't impose any form of behavior. So an implementation may extend
> the behavior as it pleases. (Otherwise these standards have a saying
> "it is unspecified whether ... or not ...")
> 
> Now, failing early is certainly a good property when we can expect
> just that; any application that uses the call in that way *will* in
> fact fail early. This is particularly important in code that otherwise
> will have a performance penalty for doing checks.

The goal is not to avoid a performance penalty but to avoid
propagating an error to a point where it's potentially hard to
diagnose or worse.

> Another acceptable strategy, IMPOV, is to forward errors where this is
> easy to do and the check doesn't impose an unacceptable penalty. The
> application then can handle the error (or not).
> 
> setenv is certainly borderline. Code for which it is performance
> critical is almost certainly broken in many ways, and on the other
> hand failures in the way we have seen here can be rare and late.

What should happen, though? Should it set TZ to blank? Unset it? Leave
it alone? If the program proceeds with a behavior contrary to the
programmer's intent, it's going to do the wrong thing.

> So I would have a small preference for being nice: do nothing to the
> environment and return an error.

I have not read the full hwclock code in question but I suspect that
the third option, "leave it alone" (and fail with EINVAL) might cause
hwclock to treat the RTC-stored time incorrectly (e.g. as localtime
rather than UTC) which would be rather bad. In other situations it
might allow a dangerous env var that was intended to be cleared to
pass through.

An additional danger of assigning a behavior to cases like this is
that the implementation becomes invalid, and had to change, breaking
programs that were depending upon the behavior, if a future version of
the standard mandates a particular behavior where the behavior was
previously undefined.

Rich

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

[Rich Felker]

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

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

[Laurent Bercot]

On 23/04/2015 11:52, Raphael Cohn wrote:
> The worst outage of my
> career was a terracotta cluster that had suffered from a split brain.
> Restarting it naively caused it to _delete_ the only remaining good
> state. Is this your 'clean state' caveat above?

  Yes it is. I think scripts that start complex services should always
make sure  to check the state first. Ideally, a run script (as we name
them in the process supervision world) should be idempotent in all cases.
I realize it's not always easy to achieve, but I believe it is the goal
to aim for.

-- 
  Laurent

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 7045 bytes --]

-----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@safe.ca 
> <mailto:jmp@safe.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@safe.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-----


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

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

Hello...

[..]
>> 
>> 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.

Hmmm... glibc-2.21, setenv.c explicitly check the value NULL
condition, so situation is checked, you could object about
the way program handle it, but it is not a bug (situation
expected and addressed).


> 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.
Fully agreed, this is pure garbage, out of spec. library
will/must crash (protect the hardware).
> 
>> (:-} why bother to return an error, just crash for all problems
>> in open, close, write, etc. just bringing the crashing concept to
>> the extreme :-}).
> 

> 
> int foo(const char *fn) { char buf[2]; strcpy(buf, "hello world,
> and goodbye");

In perfect world, strcpy should crash on the spot not going further.
this is clearly "out of spec" (a far fetch to be UB).


> int fd = open(fn ? fn : buf, O_RDONLY); return fd < 0 ? -1 : fd; }



- -- 

A bientôt
===========================================================
Jean-Marc Pigeon                        E-Mail: jmp@safe.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/

iQIcBAEBAgAGBQJVOOyKAAoJEAtgwJ4bBQU5E4YP/1G+J2vij9VF34tfZycG3iHO
Gs6y2WYsvyVZK66WsnDOOoaBr5h6bJv8pga++6hu/DVPpbXfZpY1ysNKQ63hxP02
1fFA8h1UjDJ4BM/aXHGothjp2kIXVn5CzJKuf2KUrFiR59t9WJu4KDR6/MsbRXTx
MsnOS25x+UCo4yQExemTuANAofdrst5mDyXovBdEaEGuGiTZuZri9NvZ8BZGEd4Y
v8cX/+UyY3X7UrB1+AAXbMEDs+miibXIGBI6EPPme5sDeVTm3V4kmqgdPu3Q+D00
Mgq6D+uj7Y3Wg8yHfzlVw/wZBYh1KGJNjOXJhKjmBvquCa3SQADAWBPc109J1Iip
v6efkBy3b36uWT5xoRtX4Db8+jdUUiVLLj4zRBytgkLokK8imvLlIGa1oBIUYGN8
RzC7Ma0hgHl7Tqxa7awvOfoVgqTWxC6saLmzJ+N8hFy5yg6YJYr+yvLY8fLkRPQM
BdNu/YzwFb1gIKDyq+lPtCjvMZVHpfcZMYorcV22N2zyaW7UTqU7BqRLR2mY/ojh
Nf8mUZNEsfeSNNHANZa5gixXFfDIQ/8zkZbIbY3XItESjdJft0wBmLBk8t0IJ6nO
zexIcNM3NozIZotsL+L47cHoedrYSLp/v2HK5kJ/V638qRGKhxkz9IBbe1UMwOfV
0kG3h7pAHZ5iBchsiQ8G
=yBbf
-----END PGP SIGNATURE-----


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Szabolcs Nagy]

* Jean-Marc Pigeon <jmp@safe.ca> [2015-04-23 08:58:50 -0400]:
> > 
> >> 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.
> 
> Hmmm... glibc-2.21, setenv.c explicitly check the value NULL
> condition, so situation is checked, you could object about
> the way program handle it, but it is not a bug (situation
> expected and addressed).
> 

you are wrong, glibc actually corrupted then environ on NULL
argument and this was fixed recently

http://sourceware.org/git/?p=glibc.git;a=commit;h=03c1e456b079929a8290aeb4aadb05c0df73bfd2

stop mixing runtime failure with ub, that leads to nonsense
discussions.

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

[Jean-Marc Pigeon]

[-- Attachment #1: Type: text/plain, Size: 1910 bytes --]

On 04/23/2015 09:22 AM, Szabolcs Nagy wrote:
> * Jean-Marc Pigeon <jmp@safe.ca> [2015-04-23 08:58:50 -0400]:
>>> 
>>>> 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.
>> 
>> Hmmm... glibc-2.21, setenv.c explicitly check the value NULL 
>> condition, so situation is checked, you could object about the
>> way program handle it, but it is not a bug (situation expected
>> and addressed).
>> 
> 
> you are wrong, glibc actually corrupted then environ on NULL 
> argument and this was fixed recently
> 
> http://sourceware.org/git/?p=glibc.git;a=commit;h=03c1e456b079929a8290aeb4aadb05c0df73bfd2
>
>  stop mixing runtime failure with ub, that leads to nonsense 
> discussions.
> 
glibc-2.12-1.132.el6_5.2.x86_64
I do not confirm env corruption, try:

#include        <stdio.h>

main()

{
#define ENVNAME "BIGRE"

(void) setenv(ENVNAME,getenv("MISSING_FROM_ENV"),1);
(void) fprintf(stdout,"1 env %s=<%s>\n",ENVNAME,getenv(ENVNAME));
(void) setenv(ENVNAME,"",1);
(void) fprintf(stdout,"2 env %s=<%s>\n",ENVNAME,getenv(ENVNAME));
(void) setenv(ENVNAME,"something",1);
(void) fprintf(stdout,"3 env %s=<%s>\n",ENVNAME,getenv(ENVNAME));
}

What I am trying to explain, you have something not
defined in spec, was resolved in a way (you may not like, I agree)
for EONs.

Now "you" decide, to resolve UB another way, you may
have tons of applications in jeopardy.



-- 

A bientôt
===========================================================
Jean-Marc Pigeon                        E-Mail: jmp@safe.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">
===========================================================


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4242 bytes --]

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

[Szabolcs Nagy]

* Jean-Marc Pigeon <jmp@safe.ca> [2015-04-23 09:58:28 -0400]:
> On 04/23/2015 09:22 AM, Szabolcs Nagy wrote:
> > 
> > you are wrong, glibc actually corrupted then environ on NULL 
> > argument and this was fixed recently
> > 
> > http://sourceware.org/git/?p=glibc.git;a=commit;h=03c1e456b079929a8290aeb4aadb05c0df73bfd2
> >
> >  stop mixing runtime failure with ub, that leads to nonsense 
> > discussions.
> > 
> glibc-2.12-1.132.el6_5.2.x86_64
> I do not confirm env corruption, try:
> 

==6225== Invalid read of size 1
==6225==    at 0x4C2BFC2: __GI_strlen (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6225==    by 0x4EA1F6B: puts (ioputs.c:37)
==6225==    by 0x40059D: main (in /data/tmp/env/a.out)

> 
> What I am trying to explain, you have something not
> defined in spec, was resolved in a way (you may not like, I agree)
> for EONs.
> 

never worked

bsd always crashed

glibc used to generate invalid memory access which can lead to
arbitrarily bad behaviour, now it's fixed to crash

musl always crashed

> Now "you" decide, to resolve UB another way, you may
> have tons of applications in jeopardy.
> 

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

[Isaac Dunham]

On Wed, Apr 22, 2015 at 10:15:07PM -0400, Rich Felker wrote:
> On Wed, Apr 22, 2015 at 09:26:57PM -0400, Jean-Marc Pigeon wrote:
> > 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.

I sent a patch that checks that getenv("TZUTC") is not null before using
it to the util-linux, and one of the developers proposed a different fix:
get rid of TZUTC usage altogether.

Apparently, this was introduced in 2013 by someone who wanted to use
the "right" (zoneinfo-leaps) timezone database, but didn't understand
the correct way to set it up. It happens to have been a drive-by patch
that had no updates for the public documentation, for what that's worth.

Instead of setting TZUTC (presumably to ":right/UTC" or equivalent),
the *proper* approach to using the "right" database is to set TZDIR
to /usr/share/zoneinfo/right or /usr/share/zoneinfo-leaps (depending
on your distro's packaging of tzdata), or to copy all the files into
/usr/share/zoneinfo/.
According to hwclock(8), the tzdata maintainers moved
/usr/share/zoneinfo/right to /usr/share/zoneinfo-leaps in order to
discourage people from using TZ=:right/...; this format makes it
inconvenient to mix zones on a single machine, since you cannot mix
and match "right" and "posix" without getting unexpeccted results.

tl;dr: it's not old code but a recent (2-year old) undocumented addition,
apparently done without checking the proper way to set up tzdata with
leap seconds. It is thus redundant with TZDIR, besides relying on a use
of TZ that is conceptually broken.
If someone throws together quick hacks without reading the documentation,
breakage may be expected even if they test it, since there's no guarantee
that everything everywhere will work like the few test systems one person
has for an indefinate length of time.

Thanks,
Isaac Dunham