Patchwork [vtv,update] Fix /tmp directory issues in libvtv

login
register
mail settings
Submitter Florian Weimer
Date Aug. 20, 2013, 6:35 p.m.
Message ID <5213B6DC.4040608@redhat.com>
Download mbox | patch
Permalink /patch/268637/
State New
Headers show

Comments

Florian Weimer - Aug. 20, 2013, 6:35 p.m.
On 08/20/2013 12:24 AM, Caroline Tice wrote:
> Hi All,
>
> I could really use some help here from someone who has a better
> understanding of how the config/Makefile system works than I do.
>
> In my libvtv/configure.ac file, I have:
>
> AC_GNU_SOURCE
> AC_CHECK_FUNCS([__secure_getenv])
>
> AC_GNU_SOURCE
> AC_CHECK_FUNCS([secure_getenv])

There's nothing in Makefile.am that makes use of the substitution 
variables, so configure doesn't actually emit code to define the 
matching preprocessor macros.

I tried to change that, by putting "DEFS = @DEFS@" into 
libvtv/Makefile.am (libvtv/Makefile.in is derived from that).  The diff 
below is slightly garbled, but you get the idea.



But I ended up with:

libtool: compile:  /home/fw/src/gnu/gcc/build/./gcc/xgcc 
-B/home/fw/src/gnu/gcc/build/./gcc/ "-DPACKAGE_NAME=\"GNU Vtable 
Verification Runtime Library\"" -DPACKAGE_TARNAME=\"libvtv\" 
-DPACKAGE_VERSION=\"1.0\" "-DPACKAGE_STRING=\"GNU Vtable Verification 
Runtime Library 1.0\"" -DPACKAGE_BUGREPORT=\"\" 
-DPACKAGE_URL=\"http://www.gnu.org/software/libvtv/\" 
-DPACKAGE=\"libvtv\" -DVERSION=\"1.0\" -DSTDC_HEADERS=1 
-DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_STDLIB_H=1 
-DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -D__EXTENSIONS__=1 -D_ALL_SOURCE=1 
-D_GNU_SOURCE=1 -D_POSIX_PTHREAD_SEMANTICS=1 -D_TANDEM_SOURCE=1 
-DHAVE___SECURE_GETENV=1 -I. -I../../../git/libvtv 
-I../../../git/libvtv/../include -D_GNU_SOURCE -Wall -Wextra 
-fno-exceptions -Wl,-u_vtable_map_vars_start,-u_vtable_map_vars_end -g 
-O2 -D_GNU_SOURCE -MT vtv_rts.lo -MD -MP -MF .deps/vtv_rts.Tpo -c 
../../../git/libvtv/vtv_rts.cc  -fPIC -DPIC -o .libs/vtv_rts.o
../../../git/libvtv/vtv_rts.cc:134:28: fatal error: bits/c++config.h: No 
such file or directory
  #include <bits/c++config.h>
                             ^
(The caret is at the wrong position, IMHO.)

But I suspect something went wrong with regenerating Makefile.in.  Have 
you got automake 1.11?  Then you could make the change, regenerate 
Makefile.in, and try again.
Caroline Tice - Aug. 20, 2013, 7:15 p.m.
That fixed it, thanks!   Attached is the latest patch (Florian, I will
send you the regenerated Makefile.in and configure separately).
Please review and let me know if this is OK to commit!

-- Caroline
cmtice@google.com

2013-08-20  Caroline Tice  <cmtice@google.com>

        * Makefile.am (DEFS): Add "@DEFS@", to inherit defintions.
        * Makefile.in: Regenerate.
        * configure.ac: Add check for __secure_getenv and secure_getenv.
        * configure: Regenerate.
        * vtv_utils.cc : Include stdlib.h
        (HAVE_SECURE_GETENV): Add checks and definitions for secure_getenv.
        (log_dirs): Remove file static constant.
        (__vtv_open_log):  Increase size of log file name.  Add the user
        and process ids to the file name. Do not put the log files in /tmp.
        Instead try to get the directory name from an environment variable; if
        that fails try to use stderr.  Add O_NOFOLLOW to the flags
        for 'open'.  Update function comment.
        * vtv_rts.cc (log_memory_protection_data):  Remove %d from file name.


On Tue, Aug 20, 2013 at 11:35 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/20/2013 12:24 AM, Caroline Tice wrote:
>>
>> Hi All,
>>
>> I could really use some help here from someone who has a better
>> understanding of how the config/Makefile system works than I do.
>>
>> In my libvtv/configure.ac file, I have:
>>
>> AC_GNU_SOURCE
>> AC_CHECK_FUNCS([__secure_getenv])
>>
>> AC_GNU_SOURCE
>> AC_CHECK_FUNCS([secure_getenv])
>
>
> There's nothing in Makefile.am that makes use of the substitution variables,
> so configure doesn't actually emit code to define the matching preprocessor
> macros.
>
> I tried to change that, by putting "DEFS = @DEFS@" into libvtv/Makefile.am
> (libvtv/Makefile.in is derived from that).  The diff below is slightly
> garbled, but you get the idea.
>
> diff --git a/libvtv/Makefile.am b/libvtv/Makefile.am
> index 73acfb4..567bd81 100644
> --- a/libvtv/Makefile.am
> +++ b/libvtv/Makefile.am
> @@ -30,7 +30,7 @@ ACLOCAL_AMFLAGS = -I .. -I ../config
>  # May be used by toolexeclibdir.
>  gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)
>
> -DEFS =
> +DEFS = @DEFS@
>  AM_CPPFLAGS = -I$(top_srcdir)/../include
>  AM_CFLAGS = $(XCFLAGS)
>  AM_CCASFLAGS = $(XCFLAGS)
>
>
> But I ended up with:
>
> libtool: compile:  /home/fw/src/gnu/gcc/build/./gcc/xgcc
> -B/home/fw/src/gnu/gcc/build/./gcc/ "-DPACKAGE_NAME=\"GNU Vtable
> Verification Runtime Library\"" -DPACKAGE_TARNAME=\"libvtv\"
> -DPACKAGE_VERSION=\"1.0\" "-DPACKAGE_STRING=\"GNU Vtable Verification
> Runtime Library 1.0\"" -DPACKAGE_BUGREPORT=\"\"
> -DPACKAGE_URL=\"http://www.gnu.org/software/libvtv/\" -DPACKAGE=\"libvtv\"
> -DVERSION=\"1.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1
> -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1
> -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1
> -DLT_OBJDIR=\".libs/\" -D__EXTENSIONS__=1 -D_ALL_SOURCE=1 -D_GNU_SOURCE=1
> -D_POSIX_PTHREAD_SEMANTICS=1 -D_TANDEM_SOURCE=1 -DHAVE___SECURE_GETENV=1 -I.
> -I../../../git/libvtv -I../../../git/libvtv/../include -D_GNU_SOURCE -Wall
> -Wextra -fno-exceptions -Wl,-u_vtable_map_vars_start,-u_vtable_map_vars_end
> -g -O2 -D_GNU_SOURCE -MT vtv_rts.lo -MD -MP -MF .deps/vtv_rts.Tpo -c
> ../../../git/libvtv/vtv_rts.cc  -fPIC -DPIC -o .libs/vtv_rts.o
> ../../../git/libvtv/vtv_rts.cc:134:28: fatal error: bits/c++config.h: No
> such file or directory
>  #include <bits/c++config.h>
>                             ^
> (The caret is at the wrong position, IMHO.)
>
> But I suspect something went wrong with regenerating Makefile.in.  Have you
> got automake 1.11?  Then you could make the change, regenerate Makefile.in,
> and try again.
>
>
> --
> Florian Weimer / Red Hat Product Security Team
Florian Weimer - Aug. 20, 2013, 7:43 p.m.
On 08/20/2013 09:15 PM, Caroline Tice wrote:
> That fixed it, thanks!   Attached is the latest patch (Florian, I will
> send you the regenerated Makefile.in and configure separately).
> Please review and let me know if this is OK to commit!

As the libvtv reviewer, you don't need permission to commit your 
changes. :-)

But I bootstrapped on x86_64-debian-linux-gnu, and can confirm that this 
now works as intended; __secure_getenv is picked up.

One minor nit:

Index: libvtv/vtv_utils.cc
===================================================================
--- libvtv/vtv_utils.cc	(revision 201802)
+++ libvtv/vtv_utils.cc	(working copy)

+   decriptor.

There's a stray space at the end of this line.
Caroline Tice - Aug. 20, 2013, 8:43 p.m.
Ok, committed (with the space fix).

-- Caroline Tice
cmtice@google.com

On Tue, Aug 20, 2013 at 12:43 PM, Florian Weimer <fweimer@redhat.com> wrote:
> On 08/20/2013 09:15 PM, Caroline Tice wrote:
>>
>> That fixed it, thanks!   Attached is the latest patch (Florian, I will
>> send you the regenerated Makefile.in and configure separately).
>> Please review and let me know if this is OK to commit!
>
>
> As the libvtv reviewer, you don't need permission to commit your changes.
> :-)
>
> But I bootstrapped on x86_64-debian-linux-gnu, and can confirm that this now
> works as intended; __secure_getenv is picked up.
>
> One minor nit:
>
> Index: libvtv/vtv_utils.cc
> ===================================================================
> --- libvtv/vtv_utils.cc (revision 201802)
> +++ libvtv/vtv_utils.cc (working copy)
>
> +   decriptor.
>
> There's a stray space at the end of this line.
>
>
> --
> Florian Weimer / Red Hat Product Security Team
Gerald Pfeifer - Aug. 25, 2013, 7:33 p.m.
On Tue, 20 Aug 2013, Florian Weimer wrote:
> As the libvtv reviewer, you don't need permission to commit your 
> changes. :-)

Actually, reviewers do need someone else's approval for their own
changes (unlike maintainers and of course not for trivial changes).

Not a biggie in this case, just wanted to clarify.

Gerald
Florian Weimer - Aug. 26, 2013, 9:10 p.m.
On 08/25/2013 09:33 PM, Gerald Pfeifer wrote:
> On Tue, 20 Aug 2013, Florian Weimer wrote:
>> As the libvtv reviewer, you don't need permission to commit your
>> changes. :-)
>
> Actually, reviewers do need someone else's approval for their own
> changes (unlike maintainers and of course not for trivial changes).

Ah, but I'm not a global reviewer, so I couldn't approve the change 
anyway. :-)

Caroline, I think your patch in

<http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00619.html>

doesn't match the ChangeLog update and what was approved by Jeff.  It 
should have gone into the "Various Maintainers" section.

Patch

diff --git a/libvtv/Makefile.am b/libvtv/Makefile.am
index 73acfb4..567bd81 100644
--- a/libvtv/Makefile.am
+++ b/libvtv/Makefile.am
@@ -30,7 +30,7 @@  ACLOCAL_AMFLAGS = -I .. -I ../config
  # May be used by toolexeclibdir.
  gcc_version := $(shell cat $(top_srcdir)/../gcc/BASE-VER)

-DEFS =
+DEFS = @DEFS@
  AM_CPPFLAGS = -I$(top_srcdir)/../include
  AM_CFLAGS = $(XCFLAGS)
  AM_CCASFLAGS = $(XCFLAGS)