Patchwork question about env and function malloc

login
register
mail settings
Submitter Alexandre Oliva
Date Dec. 18, 2014, 5:44 a.m.
Message ID <ortx0t7brp.fsf@free.home>
Download mbox | patch
Permalink /patch/4334/
State New
Headers show

Comments

Alexandre Oliva - Dec. 18, 2014, 5:44 a.m.
[Cc:ing the list]

On Dec 17, 2014, MaShimiao <mashimiao.fnst@cn.fujitsu.com> wrote:

> But, the description of env in glibc manual says if a function accesses 
> environment with getenv() of similar, without any guard, then it should be
> marked with env. it doesn't mention any special functions.
> Do you think we should add some words to remind users that there are some special
> conditions in which even a function calls getenv(), it will not be marked with env.
> Just like malloc() or free().
> Or we just add explanations in the description of each special functions?

I think a note at the exception point should be enough, as in the patch
below.

Ok to install?


for ChangeLog

	* manual/memory.texi (malloc): Elaborate rationale to drop
	@mtsenv from malloc_printerr.
---
 manual/memory.texi |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Ma Shimiao - Dec. 19, 2014, 1:35 a.m.
On 12/18/2014 01:44 PM, Alexandre Oliva wrote:
> I think a note at the exception point should be enough, as in the patch
> below.
> 
> Ok to install?
> 
> 
> for ChangeLog
> 
> 	* manual/memory.texi (malloc): Elaborate rationale to drop
> 	@mtsenv from malloc_printerr.
LGTM.


Best regards
Rich Felker - Dec. 19, 2014, 4:47 a.m.
On Thu, Dec 18, 2014 at 03:44:10AM -0200, Alexandre Oliva wrote:
> [Cc:ing the list]
> 
> On Dec 17, 2014, MaShimiao <mashimiao.fnst@cn.fujitsu.com> wrote:
> 
> > But, the description of env in glibc manual says if a function accesses 
> > environment with getenv() of similar, without any guard, then it should be
> > marked with env. it doesn't mention any special functions.
> > Do you think we should add some words to remind users that there are some special
> > conditions in which even a function calls getenv(), it will not be marked with env.
> > Just like malloc() or free().
> > Or we just add explanations in the description of each special functions?
> 
> I think a note at the exception point should be enough, as in the patch
> below.

This is a conformance issue; for the purpose of safety with respect to
modifying the environment in a multi-threaded program, the set of
standard functions allowed to access the environment is fixed by POSIX
and does not include malloc. The only conforming way functions outside
this fixed set can use the environment is to probe it during program
initialization (before any application-level code runs) and save the
result.

Rich
Alexandre Oliva - Dec. 22, 2014, 6:29 p.m.
On Dec 19, 2014, Rich Felker <dalias@libc.org> wrote:

> On Thu, Dec 18, 2014 at 03:44:10AM -0200, Alexandre Oliva wrote:
>> [Cc:ing the list]
>> 
>> On Dec 17, 2014, MaShimiao <mashimiao.fnst@cn.fujitsu.com> wrote:
>> 
>> > But, the description of env in glibc manual says if a function accesses 
>> > environment with getenv() of similar, without any guard, then it should be
>> > marked with env. it doesn't mention any special functions.
>> > Do you think we should add some words to remind users that there are some special
>> > conditions in which even a function calls getenv(), it will not be marked with env.
>> > Just like malloc() or free().
>> > Or we just add explanations in the description of each special functions?
>> 
>> I think a note at the exception point should be enough, as in the patch
>> below.

> This is a conformance issue; for the purpose of safety with respect to
> modifying the environment in a multi-threaded program, the set of
> standard functions allowed to access the environment is fixed by POSIX
> and does not include malloc.

The “env” safety note quite often indicate deviations from the standard
anyway, so I don't see what light your statement sheds on the issue at
hand.  Can you please elaborate?  TIA,
Rich Felker - Dec. 22, 2014, 6:30 p.m.
On Mon, Dec 22, 2014 at 04:29:17PM -0200, Alexandre Oliva wrote:
> On Dec 19, 2014, Rich Felker <dalias@libc.org> wrote:
> 
> > On Thu, Dec 18, 2014 at 03:44:10AM -0200, Alexandre Oliva wrote:
> >> [Cc:ing the list]
> >> 
> >> On Dec 17, 2014, MaShimiao <mashimiao.fnst@cn.fujitsu.com> wrote:
> >> 
> >> > But, the description of env in glibc manual says if a function accesses 
> >> > environment with getenv() of similar, without any guard, then it should be
> >> > marked with env. it doesn't mention any special functions.
> >> > Do you think we should add some words to remind users that there are some special
> >> > conditions in which even a function calls getenv(), it will not be marked with env.
> >> > Just like malloc() or free().
> >> > Or we just add explanations in the description of each special functions?
> >> 
> >> I think a note at the exception point should be enough, as in the patch
> >> below.
> 
> > This is a conformance issue; for the purpose of safety with respect to
> > modifying the environment in a multi-threaded program, the set of
> > standard functions allowed to access the environment is fixed by POSIX
> > and does not include malloc.
> 
> The “env” safety note quite often indicate deviations from the standard
> anyway, so I don't see what light your statement sheds on the issue at
> hand.  Can you please elaborate?  TIA,

OK, if this is a known issue that's being left for later, then my
comment doesn't really add anything.

Rich

Patch

diff --git a/manual/memory.texi b/manual/memory.texi
index 0729e70..03242c3 100644
--- a/manual/memory.texi
+++ b/manual/memory.texi
@@ -376,9 +376,9 @@  this function is in @file{stdlib.h}.
 @c   fastbin_index ok
 @c   fastbin ok
 @c   catomic_compare_and_exhange_val_acq ok
-@c   malloc_printerr dup @mtsenv
-@c     if we get to it, we're toast already, undefined behavior must have
-@c     been invoked before
+@c   malloc_printerr dup ok [no @mtsenv, see below]
+@c     if we get to malloc_printerr, undefined behavior must have been
+@c     invoked, so don't bother propagating @mtsenv up the call chain
 @c    libc_message @mtsenv [no leaks with cancellation disabled]
 @c     FATAL_PREPARE ok
 @c      pthread_setcancelstate disable ok