[PATCH v2 0/3] A few backlogged fixes, and a crash bug

[PATCH v2 0/3] A few backlogged fixes, and a crash bug

[Nick Alcock]

This is the same as the built-up patch series I sent in mid-December,
but without the stalled libtool fixes still awaiting review.  In
addition, this fixes a crash bug recently spotted on 32-bit Linux (and
possible, though not terribly likely, on any platform when CTF is linked
together).  Since the crash involves accessing and then (if you're
unlucky) modifying memory out of bounds, I consider it important enough to
backport to every branch with the affected code: 2.36 and later.

The usual test cycle is ongoing. No failures anywhere yet. The testsuite
is now additionally being run on the compile farm i686-pc-linux-gnu
system that reliably reproduces the overrun bug, and with
MALLOC_PERTURB_ set, though since that doesn't perturb never-used memory
I don't expect it to reliably detect that bug on its own.

Nick Alcock (3):
  ctf: fix various dreadful typos in the ctf_archive format comments
  libctf: skip the testsuite from inside dejagnu
  libctf: ctf-link outdated input check faulty

 include/ctf.h                       |  8 +--
 libctf/Makefile.am                  |  3 +-
 libctf/Makefile.in                  | 95 +++++++++++++----------------
 libctf/configure                    | 21 +++----
 libctf/configure.ac                 |  7 ++-
 libctf/ctf-link.c                   | 35 +++++++++--
 libctf/testsuite/config/default.exp |  5 ++
 7 files changed, 94 insertions(+), 80 deletions(-)

-- 
2.39.0.267.g7648178303

[PATCH v2 1/3] ctf: fix various dreadful typos in the ctf_archive format comments

[Nick Alcock]

When defining a format it helps to a) get the endianness right when you
explicitly state what it is and b) define things in terms of fields that
exist rather than fields that don't.

(A bunch of changes of names during implementation were not reflected in
these comments...)

Thanks to Jose "Eye of the Eagle" Marchesi for spotting these.

include/
	* ctf.h (struct ctf_archive) [ctfa_ctfs]: The size element of
	this is in little-endian byte order, not network byte order.
	(struct ctf_archive_modent): This is positioned right after the
	end fo the struct ctf_archive, not at the offset of a
	nonexistent field.  The number of elements in the array depends
	on ctfa_ndicts, not another nonexistent field.
---
 include/ctf.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/ctf.h b/include/ctf.h
index 4414cb0ebed..4263799f82d 100644
--- a/include/ctf.h
+++ b/include/ctf.h
@@ -599,13 +599,13 @@ struct ctf_archive
   /* Offset of the name table.  */
   uint64_t ctfa_names;
 
-  /* Offset of the CTF table.  Each element starts with a size (a uint64_t
-     in network byte order) then a ctf_dict_t of that size.  */
+  /* Offset of the CTF table.  Each element starts with a size (a little-
+     endian uint64_t) then a ctf_dict_t of that size.  */
   uint64_t ctfa_ctfs;
 };
 
-/* An array of ctfa_nnamed of this structure lies at
-   ctf_archive[ctf_archive->ctfa_modents] and gives the ctfa_ctfs or
+/* An array of ctfa_ndicts of this structure lies at
+   ctf_archive[sizeof(struct ctf_archive)] and gives the ctfa_ctfs or
    ctfa_names-relative offsets of each name or ctf_dict_t.  */
 
 typedef struct ctf_archive_modent
-- 
2.39.0.267.g7648178303

[PATCH v2 2/3] libctf: skip the testsuite from inside dejagnu

[Nick Alcock]

The libctf testsuite uses Tcl try/catch to trap run_output errors.  This
is only supported in reasonably recent Tcls, so we detect the lack of
try/catch and suppress the testsuite via an Automake conditional in its
absence.

But this turns out not to work: Automake produces a check-DEJAGNU target
regardless of the value of this conditional and sticks it in an
unconditionally-executed part of the makefile, so the testsuite gets
executed anyway, and fails with a nasty-looking syntax error.  We can't
disable it by taking "dejagnu" out of AUTOMAKE_OPTIONS, because if you
do that Automake stops you using RUNTEST, RUNTESTFLAGS and other
variables users would expect to work.

So move to disabling the testsuite from inside the testsuite itself,
importing the value of the former Automake conditional as a Tcl variable
and exiting very early in default.exp if it's false.

	* configure.ac (TCL_TRY): No longer an Automake conditional.
	Rename to...
	(HAVE_TCL_TRY): ... this.
	* Makefile.am: Drop TCL_TRY.
	(development.exp): Set have_tcl_try.
	* testsuite/config/default.exp: Exit if have_tcl_try is false.

	* configure: Regenerated.
	* Makefile.in: Likewise.
---
 libctf/Makefile.am                  |  3 +-
 libctf/Makefile.in                  | 95 +++++++++++++----------------
 libctf/configure                    | 21 +++----
 libctf/configure.ac                 |  7 ++-
 libctf/testsuite/config/default.exp |  5 ++
 5 files changed, 61 insertions(+), 70 deletions(-)

diff --git a/libctf/Makefile.am b/libctf/Makefile.am
index c4809f70bae..c959b09e492 100644
--- a/libctf/Makefile.am
+++ b/libctf/Makefile.am
@@ -72,7 +72,6 @@ EXPECT = expect
 RUNTEST = runtest
 RUNTESTFLAGS =
 
-if TCL_TRY
 CC_FOR_TARGET = ` \
   if [ -f $$r/../gcc/xgcc ] ; then \
     if [ -f $$r/../newlib/Makefile ] ; then \
@@ -105,6 +104,7 @@ check-DEJAGNU: site.exp development.exp
 development.exp: $(BFDDIR)/development.sh
 	$(AM_V_GEN)$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
 	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
+	echo "set have_tcl_try @HAVE_TCL_TRY@" >> $@
 
 # development.sh is used to determine -Werror default.
 CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
@@ -112,6 +112,5 @@ CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
 EXTRA_DEJAGNU_SITE_CONFIG = development.exp
 
 DISTCLEANFILES += site.exp development.exp
-endif
 
 include doc/local.mk
diff --git a/libctf/Makefile.in b/libctf/Makefile.in
index f2b852e3fae..c6cb55c9768 100644
--- a/libctf/Makefile.in
+++ b/libctf/Makefile.in
@@ -127,10 +127,9 @@ build_triplet = @build@
 host_triplet = @host@
 target_triplet = @target@
 @NEED_CTF_QSORT_R_TRUE@am__append_1 = ctf-qsort_r.c
-@TCL_TRY_TRUE@am__append_2 = site.exp development.exp
-@BUILD_INFO_TRUE@am__append_3 = doc/ctf-spec.texi
-@BUILD_INFO_TRUE@am__append_4 = texput.log
-@BUILD_INFO_TRUE@am__append_5 = doc/ctf-spec.info
+@BUILD_INFO_TRUE@am__append_2 = doc/ctf-spec.texi
+@BUILD_INFO_TRUE@am__append_3 = texput.log
+@BUILD_INFO_TRUE@am__append_4 = doc/ctf-spec.info
 subdir = .
 ACLOCAL_M4 = $(top_srcdir)/aclocal.m4
 am__aclocal_m4_deps = $(top_srcdir)/../bfd/acinclude.m4 \
@@ -312,7 +311,7 @@ DVIS = doc/ctf-spec.dvi
 PDFS = doc/ctf-spec.pdf
 PSS = doc/ctf-spec.ps
 HTMLS = doc/ctf-spec.html
-TEXINFOS = $(am__append_3)
+TEXINFOS = $(am__append_2)
 TEXI2DVI = texi2dvi
 TEXI2PDF = $(TEXI2DVI) --pdf --batch
 MAKEINFOHTML = $(MAKEINFO) --html
@@ -407,6 +406,7 @@ FGREP = @FGREP@
 GENCAT = @GENCAT@
 GMSGFMT = @GMSGFMT@
 GREP = @GREP@
+HAVE_TCL_TRY = @HAVE_TCL_TRY@
 INCINTL = @INCINTL@
 INSTALL = @INSTALL@
 INSTALL_DATA = @INSTALL_DATA@
@@ -522,9 +522,9 @@ ACLOCAL_AMFLAGS = -I .. -I ../config -I ../bfd
 AUTOMAKE_OPTIONS = dejagnu foreign info-in-builddir no-texinfo.tex
 
 # Variables that we might accumulate conditionally or in subdirs.
-info_TEXINFOS = $(am__append_3)
-DISTCLEANFILES = $(am__append_2) $(am__append_4)
-MAINTAINERCLEANFILES = $(am__append_5)
+info_TEXINFOS = $(am__append_2)
+DISTCLEANFILES = site.exp development.exp $(am__append_3)
+MAINTAINERCLEANFILES = $(am__append_4)
 
 # This is where we get zlib from.  zlibdir is -L../zlib and zlibinc is
 # -I../zlib, unless we were configured with --with-system-zlib, in which
@@ -559,25 +559,25 @@ libctf_la_LDFLAGS = $(libctf_ldflags_nover) @VERSION_FLAGS@
 libctf_la_SOURCES = $(libctf_nobfd_la_SOURCES) ctf-open-bfd.c
 RUNTEST = runtest
 RUNTESTFLAGS = 
-@TCL_TRY_TRUE@CC_FOR_TARGET = ` \
-@TCL_TRY_TRUE@  if [ -f $$r/../gcc/xgcc ] ; then \
-@TCL_TRY_TRUE@    if [ -f $$r/../newlib/Makefile ] ; then \
-@TCL_TRY_TRUE@      echo $$r/../gcc/xgcc -B$$r/../gcc/ -idirafter $$r/../newlib/targ-include -idirafter $${srcroot}/../newlib/libc/include -nostdinc; \
-@TCL_TRY_TRUE@    else \
-@TCL_TRY_TRUE@      echo $$r/../gcc/xgcc -B$$r/../gcc/; \
-@TCL_TRY_TRUE@    fi; \
-@TCL_TRY_TRUE@  else \
-@TCL_TRY_TRUE@    if [ "@host@" = "@target@" ] ; then \
-@TCL_TRY_TRUE@      echo $(CC); \
-@TCL_TRY_TRUE@    else \
-@TCL_TRY_TRUE@      echo gcc | sed '$(transform)'; \
-@TCL_TRY_TRUE@    fi; \
-@TCL_TRY_TRUE@  fi`
+CC_FOR_TARGET = ` \
+  if [ -f $$r/../gcc/xgcc ] ; then \
+    if [ -f $$r/../newlib/Makefile ] ; then \
+      echo $$r/../gcc/xgcc -B$$r/../gcc/ -idirafter $$r/../newlib/targ-include -idirafter $${srcroot}/../newlib/libc/include -nostdinc; \
+    else \
+      echo $$r/../gcc/xgcc -B$$r/../gcc/; \
+    fi; \
+  else \
+    if [ "@host@" = "@target@" ] ; then \
+      echo $(CC); \
+    else \
+      echo gcc | sed '$(transform)'; \
+    fi; \
+  fi`
 
 
 # development.sh is used to determine -Werror default.
-@TCL_TRY_TRUE@CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
-@TCL_TRY_TRUE@EXTRA_DEJAGNU_SITE_CONFIG = development.exp
+CONFIG_STATUS_DEPENDENCIES = $(BFDDIR)/development.sh
+EXTRA_DEJAGNU_SITE_CONFIG = development.exp
 @BUILD_INFO_TRUE@AM_MAKEINFOFLAGS = --no-split
 all: config.h
 	$(MAKE) $(AM_MAKEFLAGS) all-am
@@ -1235,18 +1235,6 @@ cscopelist-am: $(am__tagged_files)
 distclean-tags:
 	-rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags
 	-rm -f cscope.out cscope.in.out cscope.po.out cscope.files
-
-@TCL_TRY_FALSE@check-DEJAGNU: site.exp
-@TCL_TRY_FALSE@	srcdir='$(srcdir)'; export srcdir; \
-@TCL_TRY_FALSE@	EXPECT=$(EXPECT); export EXPECT; \
-@TCL_TRY_FALSE@	if $(SHELL) -c "$(RUNTEST) --version" > /dev/null 2>&1; then \
-@TCL_TRY_FALSE@	  exit_status=0; l='$(DEJATOOL)'; for tool in $$l; do \
-@TCL_TRY_FALSE@	    if $(RUNTEST) $(AM_RUNTESTFLAGS) $(RUNTESTDEFAULTFLAGS) $(RUNTESTFLAGS); \
-@TCL_TRY_FALSE@	    then :; else exit_status=1; fi; \
-@TCL_TRY_FALSE@	  done; \
-@TCL_TRY_FALSE@	else echo "WARNING: could not find '$(RUNTEST)'" 1>&2; :;\
-@TCL_TRY_FALSE@	fi; \
-@TCL_TRY_FALSE@	exit $$exit_status
 site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
 	@echo 'Making a new site.exp file ...'
 	@echo '## these variables are automatically generated by make ##' >site.tmp
@@ -1685,23 +1673,24 @@ uninstall-am: uninstall-dvi-am uninstall-html-am \
 .PRECIOUS: Makefile
 
 
-@TCL_TRY_TRUE@check-DEJAGNU: site.exp development.exp
-@TCL_TRY_TRUE@	srcroot=`cd $(srcdir) && pwd`; export srcroot; \
-@TCL_TRY_TRUE@	r=`pwd`; export r; \
-@TCL_TRY_TRUE@	LC_ALL=C; export LC_ALL; \
-@TCL_TRY_TRUE@	EXPECT=$(EXPECT); export EXPECT; \
-@TCL_TRY_TRUE@	runtest=$(RUNTEST); \
-@TCL_TRY_TRUE@	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
-@TCL_TRY_TRUE@	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
-@TCL_TRY_TRUE@		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" \
-@TCL_TRY_TRUE@		CFLAGS="$(CFLAGS) -I$(INCDIR) -I$(srcdir) -I$(builddir) -I$(builddir)/../bfd $(ZLIBINC)" \
-@TCL_TRY_TRUE@		LIBS="$(libctf_nobfd_la_LIBADD) $(LIBS)" $(RUNTESTFLAGS); \
-@TCL_TRY_TRUE@	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
-@TCL_TRY_TRUE@	fi
-
-@TCL_TRY_TRUE@development.exp: $(BFDDIR)/development.sh
-@TCL_TRY_TRUE@	$(AM_V_GEN)$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
-@TCL_TRY_TRUE@	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
+check-DEJAGNU: site.exp development.exp
+	srcroot=`cd $(srcdir) && pwd`; export srcroot; \
+	r=`pwd`; export r; \
+	LC_ALL=C; export LC_ALL; \
+	EXPECT=$(EXPECT); export EXPECT; \
+	runtest=$(RUNTEST); \
+	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
+	  $$runtest --tool $(DEJATOOL) --srcdir $${srcroot}/testsuite \
+		CC="$(CC)" CC_FOR_TARGET="$(CC_FOR_TARGET)" \
+		CFLAGS="$(CFLAGS) -I$(INCDIR) -I$(srcdir) -I$(builddir) -I$(builddir)/../bfd $(ZLIBINC)" \
+		LIBS="$(libctf_nobfd_la_LIBADD) $(LIBS)" $(RUNTESTFLAGS); \
+	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
+	fi
+
+development.exp: $(BFDDIR)/development.sh
+	$(AM_V_GEN)$(EGREP) "(development|experimental)=" $(BFDDIR)/development.sh  \
+	  | $(AWK) -F= '{ print "set " $$1 " " $$2 }' > $@
+	echo "set have_tcl_try @HAVE_TCL_TRY@" >> $@
 
 @BUILD_INFO_TRUE@html-local: doc/ctf-spec/index.html
 @BUILD_INFO_TRUE@doc/ctf-spec/index.html: doc/ctf-spec.texi doc/$(am__dirstamp)
diff --git a/libctf/configure b/libctf/configure
index 17463a74cca..c22f7dffd2c 100755
--- a/libctf/configure
+++ b/libctf/configure
@@ -637,8 +637,7 @@ LTLIBOBJS
 LIBOBJS
 VERSION_FLAGS_NOBFD
 VERSION_FLAGS
-TCL_TRY_FALSE
-TCL_TRY_TRUE
+HAVE_TCL_TRY
 EXPECT
 CTF_LIBADD
 SHARED_LDFLAGS
@@ -11632,7 +11631,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11635 "configure"
+#line 11634 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11738,7 +11737,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11741 "configure"
+#line 11740 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -14975,12 +14974,10 @@ fi`
 fi
 { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_libctf_tcl_try" >&5
 $as_echo "$ac_cv_libctf_tcl_try" >&6; }
- if test "${ac_cv_libctf_tcl_try}" = yes; then
-  TCL_TRY_TRUE=
-  TCL_TRY_FALSE='#'
-else
-  TCL_TRY_TRUE='#'
-  TCL_TRY_FALSE=
+
+HAVE_TCL_TRY=false
+if test "${ac_cv_libctf_tcl_try}" = "yes"; then
+    HAVE_TCL_TRY=true
 fi
 
 
@@ -15208,10 +15205,6 @@ if test -z "${BUILD_INFO_TRUE}" && test -z "${BUILD_INFO_FALSE}"; then
   as_fn_error $? "conditional \"BUILD_INFO\" was never defined.
 Usually this means the macro was only invoked conditionally." "$LINENO" 5
 fi
-if test -z "${TCL_TRY_TRUE}" && test -z "${TCL_TRY_FALSE}"; then
-  as_fn_error $? "conditional \"TCL_TRY\" was never defined.
-Usually this means the macro was only invoked conditionally." "$LINENO" 5
-fi
 
 : "${CONFIG_STATUS=./config.status}"
 ac_write_fail=0
diff --git a/libctf/configure.ac b/libctf/configure.ac
index a0148a4c328..1d0cf4d0fa5 100644
--- a/libctf/configure.ac
+++ b/libctf/configure.ac
@@ -241,7 +241,12 @@ if @<:@llength @<:@info commands try@:>@@:>@ then { puts yes } else { puts no }
 EOF
 fi`
 ])
-AM_CONDITIONAL(TCL_TRY, test "${ac_cv_libctf_tcl_try}" = yes)
+
+HAVE_TCL_TRY=false
+if test "${ac_cv_libctf_tcl_try}" = "yes"; then
+    HAVE_TCL_TRY=true
+fi
+AC_SUBST(HAVE_TCL_TRY)
 
 # Use a version script, if possible, or an -export-symbols-regex otherwise.
 decommented_version_script=
diff --git a/libctf/testsuite/config/default.exp b/libctf/testsuite/config/default.exp
index 4bac9c4281c..357c4ee57f1 100644
--- a/libctf/testsuite/config/default.exp
+++ b/libctf/testsuite/config/default.exp
@@ -21,6 +21,11 @@
 # Written by Jeffrey Wheat (cassidy@cygnus.com)
 #
 
+# Don't run anything if Tcl is too old to have try / catch.
+if { ! $have_tcl_try } {
+    log_and_exit
+}
+
 if ![info exists ld] then {
     set ld [findfile $base_dir/../ld/ld-new $base_dir/../ld/ld-new [transform ld]]
 }
-- 
2.39.0.267.g7648178303

[PATCH v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Alcock]

This check has a pair of faults which, combined, can lead to memory
corruption.  Firstly, it assumes that the values of the ctf_link_inputs
hash are ctf_dict_t's: they are not, they are ctf_link_input_t's, a much
shorter structure.  So the flags check which is the core of this is
faulty (but happens, by chance, to give the right output on most
architectures, since usually we happen to get a 0 here, so the test that
checks this usually passes).  Worse, the warning that is emitted when
the test fails is added to the wrong dict -- it's added to the input
dict, whose warning list is never consumed, rendering the whole check
useless.  But the dict it adds to is still the wrong type, so we end up
overwriting something deep in memory (or, much more likely,
dereferencing a garbage pointer and crashing).

Fixing both reveals another problem: the link input is an *archive*
consisting of multiple members, so we have to consider whether to check
all of them for the outdated-func-info thing we are checking here.
However, no compiler exists that emits a mixture of members with this
flag on and members with it off, and the linker always reserializes (and
upgrades) such things when it sees them: so all members in a given
archive must have the same value of the flag, so we only need to check
one member per input archive.

libctf/
	PR libctf/29983
	* ctf-link.c (ctf_link_warn_outdated_inputs): Get the types of
        members of ctf_link_inputs right, fixing a possible spurious
        tesst failure / wild pointer deref / overwrite.  Emit the
        warning message into the right dict.
---
 libctf/ctf-link.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
index 2837168b2a6..df8fa3b9d9b 100644
--- a/libctf/ctf-link.c
+++ b/libctf/ctf-link.c
@@ -1848,19 +1848,42 @@ ctf_link_warn_outdated_inputs (ctf_dict_t *fp)
 {
   ctf_next_t *i = NULL;
   void *name_;
-  void *ifp_;
+  void *input_;
   int err;
 
-  while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &ifp_)) == 0)
+  while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &input_)) == 0)
     {
       const char *name = (const char *) name_;
-      ctf_dict_t *ifp = (ctf_dict_t *) ifp_;
+      ctf_link_input_t *input = (ctf_link_input_t *) input_;
+      ctf_next_t *j = NULL;
+      ctf_dict_t *ifp;
+      int err;
+
+      /* We only care about CTF archives by this point: lazy-opened archives
+         have always been opened by this point, and short-circuited entries have
+         a matching corresponding archive member. Entries with NULL clin_arc can
+         exist, and constitute old entries renamed via a name changer: the
+         renamed entries exist elsewhere in the list, so we can just skip
+         those.  */
+
+      if (!input->clin_arc)
+        continue;
+
+      /* All entries in the archive will necessarily contain the same
+         CTF_F_NEWFUNCINFO flag, so we only need to check the first. We don't
+         even need to do that if we can't open it for any reason at all: the
+         link will fail later on regardless, since an input can't be opened. */
+
+      ifp = ctf_archive_next (input->clin_arc, &j, NULL, 0, &err);
+      if (!ifp)
+        continue;
+      ctf_next_destroy (j);
 
       if (!(ifp->ctf_header->cth_flags & CTF_F_NEWFUNCINFO)
 	  && (ifp->ctf_header->cth_varoff - ifp->ctf_header->cth_funcoff) > 0)
-	ctf_err_warn (ifp, 1, 0, _("linker input %s has CTF func info but uses "
-				   "an old, unreleased func info format: "
-				   "this func info section will be dropped."),
+	ctf_err_warn (fp, 1, 0, _("linker input %s has CTF func info but uses "
+                                  "an old, unreleased func info format: "
+                                  "this func info section will be dropped."),
 		      name);
     }
   if (err != ECTF_NEXT_END)
-- 
2.39.0.267.g7648178303

Re: [PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Alcock]

Is this patch alone OK for 2.40? (After I push this series to master.
Tomorrow, not today... all tests passed, crash on i686-pc-linux-gnu
fixed, valgrind-clean, asan-clean.)

(I plan to backport it all the way back to 2.36.)

On 10 Jan 2023, Nick Alcock via Binutils spake thusly:

> This check has a pair of faults which, combined, can lead to memory
> corruption.  Firstly, it assumes that the values of the ctf_link_inputs
> hash are ctf_dict_t's: they are not, they are ctf_link_input_t's, a much
> shorter structure.  So the flags check which is the core of this is
> faulty (but happens, by chance, to give the right output on most
> architectures, since usually we happen to get a 0 here, so the test that
> checks this usually passes).  Worse, the warning that is emitted when
> the test fails is added to the wrong dict -- it's added to the input
> dict, whose warning list is never consumed, rendering the whole check
> useless.  But the dict it adds to is still the wrong type, so we end up
> overwriting something deep in memory (or, much more likely,
> dereferencing a garbage pointer and crashing).
>
> Fixing both reveals another problem: the link input is an *archive*
> consisting of multiple members, so we have to consider whether to check
> all of them for the outdated-func-info thing we are checking here.
> However, no compiler exists that emits a mixture of members with this
> flag on and members with it off, and the linker always reserializes (and
> upgrades) such things when it sees them: so all members in a given
> archive must have the same value of the flag, so we only need to check
> one member per input archive.
>
> libctf/
> 	PR libctf/29983
> 	* ctf-link.c (ctf_link_warn_outdated_inputs): Get the types of
>         members of ctf_link_inputs right, fixing a possible spurious
>         tesst failure / wild pointer deref / overwrite.  Emit the
>         warning message into the right dict.
> ---
>  libctf/ctf-link.c | 35 +++++++++++++++++++++++++++++------
>  1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
> index 2837168b2a6..df8fa3b9d9b 100644
> --- a/libctf/ctf-link.c
> +++ b/libctf/ctf-link.c
> @@ -1848,19 +1848,42 @@ ctf_link_warn_outdated_inputs (ctf_dict_t *fp)
>  {
>    ctf_next_t *i = NULL;
>    void *name_;
> -  void *ifp_;
> +  void *input_;
>    int err;
>  
> -  while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &ifp_)) == 0)
> +  while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &input_)) == 0)
>      {
>        const char *name = (const char *) name_;
> -      ctf_dict_t *ifp = (ctf_dict_t *) ifp_;
> +      ctf_link_input_t *input = (ctf_link_input_t *) input_;
> +      ctf_next_t *j = NULL;
> +      ctf_dict_t *ifp;
> +      int err;
> +
> +      /* We only care about CTF archives by this point: lazy-opened archives
> +         have always been opened by this point, and short-circuited entries have
> +         a matching corresponding archive member. Entries with NULL clin_arc can
> +         exist, and constitute old entries renamed via a name changer: the
> +         renamed entries exist elsewhere in the list, so we can just skip
> +         those.  */
> +
> +      if (!input->clin_arc)
> +        continue;
> +
> +      /* All entries in the archive will necessarily contain the same
> +         CTF_F_NEWFUNCINFO flag, so we only need to check the first. We don't
> +         even need to do that if we can't open it for any reason at all: the
> +         link will fail later on regardless, since an input can't be opened. */
> +
> +      ifp = ctf_archive_next (input->clin_arc, &j, NULL, 0, &err);
> +      if (!ifp)
> +        continue;
> +      ctf_next_destroy (j);
>  
>        if (!(ifp->ctf_header->cth_flags & CTF_F_NEWFUNCINFO)
>  	  && (ifp->ctf_header->cth_varoff - ifp->ctf_header->cth_funcoff) > 0)
> -	ctf_err_warn (ifp, 1, 0, _("linker input %s has CTF func info but uses "
> -				   "an old, unreleased func info format: "
> -				   "this func info section will be dropped."),
> +	ctf_err_warn (fp, 1, 0, _("linker input %s has CTF func info but uses "
> +                                  "an old, unreleased func info format: "
> +                                  "this func info section will be dropped."),
>  		      name);
>      }
>    if (err != ECTF_NEXT_END)

-- 
NULL && (void)

Re: [PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Clifton]

Hi Nick,

> Is this patch alone OK for 2.40?

It is - but please do this today if you can - I am planning on creating
the release tarballs tomorrow...

> (After I push this series to master.
> Tomorrow, not today... all tests passed, crash on i686-pc-linux-gnu
> fixed, valgrind-clean, asan-clean.)
> 
> (I plan to backport it all the way back to 2.36.)

Have fun!

Cheers
   Nick

Re: [PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Alcock]

On 12 Jan 2023, Nick Clifton via Binutils said:

> Hi Nick,
>
>> Is this patch alone OK for 2.40?
>
> It is - but please do this today if you can - I am planning on creating
> the release tarballs tomorrow...

OK! I'll be pushing to master and 2.40 in twenty minutes or so (final
paranoid tests running). A bit longer for the earlier branches, but
they're not holding up a release :)

-- 
NULL && (void)

Re: [PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Alcock]

On 12 Jan 2023, Nick Alcock via Binutils uttered the following:

> On 12 Jan 2023, Nick Clifton via Binutils said:
>>> Is this patch alone OK for 2.40?
>>
>> It is - but please do this today if you can - I am planning on creating
>> the release tarballs tomorrow...
>
> OK! I'll be pushing to master and 2.40 in twenty minutes or so (final

Hark at the optimism! I reckoned without the traditional last-minute
massive slowdown of every system one is testing on. I'm only lucky the
power didn't go out.

> paranoid tests running). A bit longer for the earlier branches, but
> they're not holding up a release :)

Tests passed, finally: pushed to master and 2.40! Sorry this was so
late.

-- 
NULL && (void)

Re: [PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty

[Nick Clifton]

Hi Nick,

>>> It is - but please do this today if you can - I am planning on creating
>>> the release tarballs tomorrow...
>>
>> OK! I'll be pushing to master and 2.40 in twenty minutes or so (final
> 
> Hark at the optimism! I reckoned without the traditional last-minute
> massive slowdown of every system one is testing on. I'm only lucky the
> power didn't go out.

Ha- it is always the way.  I must apologise too, since I was wrong to
say that I am going to releasing today (Fri 13th).  I am actually
planning to make the release tomorrow (Sat 14th), but when I wrote
the original email I had it in my head that it was Friday already.
Wishful thinking I suppose...

> Tests passed, finally: pushed to master and 2.40!

Yay!

Cheers
   Nick