libctf: fix creation-time parent/child dict confusions

The fixes applied a few years ago to resolve confusions between parent and
child dicts at lookup time also apply in various forms to creation.  In
general, if you have a type in a parent dict ctf_imported into a child and
you do something to it, and the parent dict is writable (created via
ctf_create, not opened via ctf_open*) it should work just the same to make
changes to that type via a child dict as it does to make the change
to the parent dict directly -- and nothing you're prohibited from doing
to the parent dict when done directly should be allowed just because
you're doing it via a child.

Specifically, the following don't work when doing things from the child, but
should:

 - adding a member of a type in the parent to a struct or union in the
   parent via ctf_add_member or ctf_add_member_offset: this yields
   ECTF_BADID

 - adding a member of a type in the parent to a struct or union in the
   parent via ctf_add_member_encoded: this dumps core (!).

 - adding an enumerand to an enumerator in the parent: this yields
   ECTF_BADID

 - setting the properties of an array in the parent via ctf_set_array;
   this yields ECTF_BADID

Relatedly, some things work when doing things via a child that should fail,
yielding a CTF dictionary with invalid content (readable, but meaningless):
in particular, you can add a child type to a struct in the parent via
any of the ctf_add_member* family and nothing complains at all, even though
you should never be able to add references to children to parents (since any
given parent can be associated with many different children).

A family of tests is added to check each of these cases independently, since
some can result in coredumps and it would be nice to test the other cases
even if some dump core.  They use a common library to do all the actual
work.  The set of affected API calls was determined by code inspection
(auditing all calls to ctf_dtd_lookup): it's possible that I missed a few,
but I doubt it, since other cases use ctf_lookup* functions, which already
climb to the parent where appropriate.

libctf/ChangeLog:

	PR libctf/30985
	* ctf-create.c (ctf_dtd_lookup): Traverse to parents if necessary.
	(ctf_set_array): Likewise.  Report errors on the child; require
	both parent and child to be writable.
	(ctf_add_enumerator): Likewise.
	(ctf_add_member_offset): Likewise.  Prohibit addition of child types
	to structs in the parent.
	(ctf_add_member_encoded): Do not dereference a NULL dtd: report
	ECTF_BADID instead.
	* ctf-string.c (ctf_str_add_ref_internal): Report ENOMEM on the
	dict if addition of a string ref fails.
	* testsuite/libctf-writable/parent-child-dtd-crash-lib.c: New library.
	* testsuite/libctf-writable/parent-child-dtd-enum.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-enumerator.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-member-encoded.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-member-offset.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-set-array.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-struct.*: New test.
	* testsuite/libctf-writable/parent-child-dtd-union.*: New test.

Author: nickalcock

libctf/ctf-create.c

@@ -299,6 +299,9 @@ ctf_dtd_delete (ctf_dict_t *fp, ctf_dtdef_t *dtd)
 ctf_dtdef_t *
 ctf_dtd_lookup (const ctf_dict_t *fp, ctf_id_t type)
 {
+  if ((fp->ctf_flags & LCTF_CHILD) && LCTF_TYPE_ISPARENT (fp, type))
+    fp = fp->ctf_parent;
+
   return (ctf_dtdef_t *)
     ctf_dynhash_lookup (fp->ctf_dthash, (void *) (uintptr_t) type);
 }
@@ -712,15 +715,22 @@ ctf_add_array (ctf_dict_t *fp, uint32_t flag, const ctf_arinfo_t *arp)
 int
 ctf_set_array (ctf_dict_t *fp, ctf_id_t type, const ctf_arinfo_t *arp)
 {
+  ctf_dict_t *ofp = fp;
   ctf_dtdef_t *dtd = ctf_dtd_lookup (fp, type);
   ctf_array_t *vlen;
 
+  if ((fp->ctf_flags & LCTF_CHILD) && LCTF_TYPE_ISPARENT (fp, type))
+    fp = fp->ctf_parent;
+
+  if (!(ofp->ctf_flags & LCTF_RDWR))
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
+
   if (!(fp->ctf_flags & LCTF_RDWR))
-    return (ctf_set_errno (fp, ECTF_RDONLY));
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
 
   if (dtd == NULL
       || LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info) != CTF_K_ARRAY)
-    return (ctf_set_errno (fp, ECTF_BADID));
+    return (ctf_set_errno (ofp, ECTF_BADID));
 
   vlen = (ctf_array_t *) dtd->dtd_vlen;
   fp->ctf_flags |= LCTF_DIRTY;
@@ -1040,6 +1050,7 @@ int
 ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
 		    int value)
 {
+  ctf_dict_t *ofp = fp;
   ctf_dtdef_t *dtd = ctf_dtd_lookup (fp, enid);
   unsigned char *old_vlen;
   ctf_enum_t *en;
@@ -1050,21 +1061,27 @@ ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
   if (name == NULL)
     return (ctf_set_errno (fp, EINVAL));
 
+  if ((fp->ctf_flags & LCTF_CHILD) && LCTF_TYPE_ISPARENT (fp, enid))
+    fp = fp->ctf_parent;
+
+  if (!(ofp->ctf_flags & LCTF_RDWR))
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
+
   if (!(fp->ctf_flags & LCTF_RDWR))
-    return (ctf_set_errno (fp, ECTF_RDONLY));
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
 
   if (dtd == NULL)
-    return (ctf_set_errno (fp, ECTF_BADID));
+    return (ctf_set_errno (ofp, ECTF_BADID));
 
   kind = LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info);
   root = LCTF_INFO_ISROOT (fp, dtd->dtd_data.ctt_info);
   vlen = LCTF_INFO_VLEN (fp, dtd->dtd_data.ctt_info);
 
   if (kind != CTF_K_ENUM)
-    return (ctf_set_errno (fp, ECTF_NOTENUM));
+    return (ctf_set_errno (ofp, ECTF_NOTENUM));
 
   if (vlen == CTF_MAX_VLEN)
-    return (ctf_set_errno (fp, ECTF_DTFULL));
+    return (ctf_set_errno (ofp, ECTF_DTFULL));
 
   old_vlen = dtd->dtd_vlen;
   if (ctf_grow_vlen (fp, dtd, sizeof (ctf_enum_t) * (vlen + 1)) < 0)
@@ -1083,13 +1100,13 @@ ctf_add_enumerator (ctf_dict_t *fp, ctf_id_t enid, const char *name,
 
   for (i = 0; i < vlen; i++)
     if (strcmp (ctf_strptr (fp, en[i].cte_name), name) == 0)
-      return (ctf_set_errno (fp, ECTF_DUPLICATE));
+      return (ctf_set_errno (ofp, ECTF_DUPLICATE));
 
   en[i].cte_name = ctf_str_add_pending (fp, name, &en[i].cte_name);
   en[i].cte_value = value;
 
   if (en[i].cte_name == 0 && name != NULL && name[0] != '\0')
-    return -1;					/* errno is set for us. */
+    return (ctf_set_errno (ofp, ctf_errno (fp)));
 
   dtd->dtd_data.ctt_info = CTF_TYPE_INFO (kind, root, vlen + 1);
 
@@ -1102,6 +1119,7 @@ int
 ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
 		       ctf_id_t type, unsigned long bit_offset)
 {
+  ctf_dict_t *ofp = fp;
   ctf_dtdef_t *dtd = ctf_dtd_lookup (fp, souid);
 
   ssize_t msize, malign, ssize;
@@ -1111,11 +1129,25 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
   unsigned char *old_vlen;
   ctf_lmember_t *memb;
 
+  if ((fp->ctf_flags & LCTF_CHILD) && LCTF_TYPE_ISPARENT (fp, souid))
+    {
+      /* Adding a child type to a parent, even via the child, is prohibited.
+	 Otherwise, climb to the parent and do all work there.  */
+
+      if (LCTF_TYPE_ISCHILD (fp, type))
+	return (ctf_set_errno (ofp, ECTF_BADID));
+
+      fp = fp->ctf_parent;
+    }
+
+  if (!(ofp->ctf_flags & LCTF_RDWR))
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
+
   if (!(fp->ctf_flags & LCTF_RDWR))
-    return (ctf_set_errno (fp, ECTF_RDONLY));
+    return (ctf_set_errno (ofp, ECTF_RDONLY));
 
   if (dtd == NULL)
-    return (ctf_set_errno (fp, ECTF_BADID));
+    return (ctf_set_errno (ofp, ECTF_BADID));
 
   if (name != NULL && name[0] == '\0')
     name = NULL;
@@ -1125,14 +1157,14 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
   vlen = LCTF_INFO_VLEN (fp, dtd->dtd_data.ctt_info);
 
   if (kind != CTF_K_STRUCT && kind != CTF_K_UNION)
-    return (ctf_set_errno (fp, ECTF_NOTSOU));
+    return (ctf_set_errno (ofp, ECTF_NOTSOU));
 
   if (vlen == CTF_MAX_VLEN)
-    return (ctf_set_errno (fp, ECTF_DTFULL));
+    return (ctf_set_errno (ofp, ECTF_DTFULL));
 
   old_vlen = dtd->dtd_vlen;
   if (ctf_grow_vlen (fp, dtd, sizeof (ctf_lmember_t) * (vlen + 1)) < 0)
-    return -1;					/* errno is set for us.  */
+    return (ctf_set_errno (ofp, ctf_errno (fp)));
   memb = (ctf_lmember_t *) dtd->dtd_vlen;
 
   if (dtd->dtd_vlen != old_vlen)
@@ -1149,7 +1181,7 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
     {
       for (i = 0; i < vlen; i++)
 	if (strcmp (ctf_strptr (fp, memb[i].ctlm_name), name) == 0)
-	  return (ctf_set_errno (fp, ECTF_DUPLICATE));
+	  return (ctf_set_errno (ofp, ECTF_DUPLICATE));
     }
 
   if ((msize = ctf_type_size (fp, type)) < 0 ||
@@ -1200,12 +1232,12 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
 
 	  if (is_incomplete)
 	    {
-	      ctf_err_warn (fp, 1, ECTF_INCOMPLETE,
+	      ctf_err_warn (ofp, 1, ECTF_INCOMPLETE,
 			    _("ctf_add_member_offset: cannot add member %s of "
 			      "incomplete type %lx to struct %lx without "
 			      "specifying explicit offset\n"),
 			    name ? name : _("(unnamed member)"), type, souid);
-	      return (ctf_set_errno (fp, ECTF_INCOMPLETE));
+	      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
 	    }
 
 	  if (ctf_type_encoding (fp, ltype, &linfo) == 0)
@@ -1216,14 +1248,14 @@ ctf_add_member_offset (ctf_dict_t *fp, ctf_id_t souid, const char *name,
 	    {
 	      const char *lname = ctf_strraw (fp, memb[vlen - 1].ctlm_name);
 
-	      ctf_err_warn (fp, 1, ECTF_INCOMPLETE,
+	      ctf_err_warn (ofp, 1, ECTF_INCOMPLETE,
 			    _("ctf_add_member_offset: cannot add member %s of "
 			      "type %lx to struct %lx without specifying "
 			      "explicit offset after member %s of type %lx, "
 			      "which is an incomplete type\n"),
 			    name ? name : _("(unnamed member)"), type, souid,
 			    lname ? lname : _("(unnamed member)"), ltype);
-	      return -1;			/* errno is set for us.  */
+	      return (ctf_set_errno (ofp, ECTF_INCOMPLETE));
 	    }
 
 	  /* Round up the offset of the end of the last member to
@@ -1274,9 +1306,14 @@ ctf_add_member_encoded (ctf_dict_t *fp, ctf_id_t souid, const char *name,
 			const ctf_encoding_t encoding)
 {
   ctf_dtdef_t *dtd = ctf_dtd_lookup (fp, type);
-  int kind = LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info);
+  int kind;
   int otype = type;
 
+  if (dtd == NULL)
+    return (ctf_set_errno (fp, ECTF_BADID));
+
+  kind = LCTF_INFO_KIND (fp, dtd->dtd_data.ctt_info);
+
   if ((kind != CTF_K_INTEGER) && (kind != CTF_K_FLOAT) && (kind != CTF_K_ENUM))
     return (ctf_set_errno (fp, ECTF_NOTINTFP));
 

libctf/ctf-string.c

@@ -170,8 +170,10 @@ ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str,
 
   if (flags & CTF_STR_ADD_REF)
     {
-      if ((aref = malloc (sizeof (struct ctf_str_atom_ref))) == NULL)
+      if ((aref = malloc (sizeof (struct ctf_str_atom_ref))) == NULL) {
+	ctf_set_errno (fp, ENOMEM);
 	return NULL;
+      }
       aref->caf_ref = ref;
     }
 

libctf/testsuite/libctf-writable/parent-child-dtd-crash-lib.c

@@ -0,0 +1,179 @@
+/* Make sure we do various things right that involve DTD lookups of parents
+   from the perspective of children.  */
+
+#include <ctf-api.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+enum crash_method { ADD_STRUCT, ADD_UNION, ADD_MEMBER_OFFSET, ADD_MEMBER_ENCODED, ADD_ENUM, ADD_ENUMERATOR, SET_ARRAY };
+
+void
+dtd_crash (enum crash_method method, int parent_bigger)
+{
+  ctf_dict_t *pfp, *cfp;
+  ctf_encoding_t e = { CTF_INT_SIGNED, 0, sizeof (long) };
+  ctf_id_t ptype, ftype, stype, foo;
+  int forward_kind = CTF_K_STRUCT;
+  size_t i;
+  int err;
+
+  /* Maybe make the relevant type IDs in the parent much bigger than those
+     in the child, or maybe vice versa.  */
+
+  if ((pfp = ctf_create (&err)) == NULL)
+    goto create_err;
+
+  if (parent_bigger)
+    {
+      if ((foo = ctf_add_integer (pfp, CTF_ADD_NONROOT, "blah", &e)) == CTF_ERR)
+	goto create_parent;
+
+      for (i = 0; i < 4096; i++)
+	if (ctf_add_pointer (pfp, CTF_ADD_NONROOT, foo) == CTF_ERR)
+	  goto create_parent;
+    }
+
+  if ((ptype = ctf_add_integer (pfp, CTF_ADD_NONROOT, "int", &e)) == CTF_ERR)
+    goto create_parent;
+
+  /* Add a forward to a struct, union, or enum (depending on the method) in
+     the parent, so we can try to replace it in the child and see what
+     happens.  (Most of them are structs, or it doesn't matter, as for
+     SET_ARRAY; so we do that by default.)  */
+
+  switch (method)
+    {
+    case ADD_UNION:
+      forward_kind = CTF_K_UNION;
+      break;
+    case ADD_ENUM:
+    case ADD_ENUMERATOR:
+      forward_kind = CTF_K_ENUM;
+      break;
+      /* Placate clang.  */
+    default:
+      break;
+    }
+
+  if ((ftype = ctf_add_forward (pfp, CTF_ADD_ROOT, "foo", forward_kind)) == CTF_ERR)
+    goto create_parent;
+
+  if ((cfp = ctf_create (&err)) == NULL)
+    goto create_err;
+
+  if (ctf_import (cfp, pfp) < 0)
+    goto create_child;
+
+  if (!parent_bigger)
+    {
+      if ((foo = ctf_add_integer (pfp, CTF_ADD_NONROOT, "blah", &e)) == CTF_ERR)
+	goto create_parent;
+
+      for (i = 0; i < 4096; i++)
+	if (ctf_add_pointer (pfp, CTF_ADD_NONROOT, foo) == CTF_ERR)
+	  goto create_parent;
+    }
+
+  switch (method)
+    {
+      /* These try to replace a forward, and should not do so if we're
+	 adding in the child and it's in the parent.  */
+    case ADD_STRUCT:
+      if ((stype = ctf_add_struct_sized (cfp, CTF_ADD_ROOT, "foo", 1024)) == CTF_ERR)
+	goto create_child;
+      if (stype == ftype)
+	fprintf (stderr, "Forward-promotion spotted!\n");
+      break;
+
+    case ADD_UNION:
+      if ((stype = ctf_add_union_sized (cfp, CTF_ADD_ROOT, "foo", 1024)) == CTF_ERR)
+	goto create_child;
+      if (stype == ftype)
+	fprintf (stderr, "Forward-promotion spotted!\n");
+      break;
+
+    case ADD_ENUM:
+      if ((stype = ctf_add_enum (cfp, CTF_ADD_ROOT, "foo")) == CTF_ERR)
+	goto create_child;
+      if (stype == ftype)
+	fprintf (stderr, "Forward-promotion spotted!\n");
+      break;
+
+      /* These try to look up the struct/union/enum we're adding to: make
+	 sure this works from the perspective of the child if the type is in
+	 the parent.  Also make sure that addition of child types to parent
+	 types this way is prohibited, and that addition of parent types to
+	 parent types is allowed.  */
+    case ADD_MEMBER_OFFSET:
+      {
+	ctf_id_t ctype;
+
+	if ((stype = ctf_add_struct (pfp, CTF_ADD_ROOT, "bar")) == CTF_ERR)
+	  goto create_child;
+
+	if ((ctype = ctf_add_integer (cfp, CTF_ADD_NONROOT, "xyzzy", &e)) == CTF_ERR)
+	  goto create_child;
+
+	if (ctf_add_member_offset (cfp, stype, "member", ptype, 5) == CTF_ERR)
+	  goto create_child;
+
+	if (ctf_add_member_offset (cfp, stype, "xyzzy", ctype, 4) != CTF_ERR)
+	  fprintf (stderr, "Addition of child type to parent via child unexpectedly succeeded\n");
+	else if (ctf_errno (cfp) == 0)
+	  fprintf (stderr, "got error from ctype addition to parent struct, but no error found on child\n");
+
+	break;
+      }
+
+    case ADD_ENUMERATOR:
+      if ((stype = ctf_add_enum (pfp, CTF_ADD_ROOT, "bar")) == CTF_ERR)
+	goto create_parent;
+
+      if (ctf_add_enumerator (cfp, stype, "FOO", 0) == CTF_ERR)
+	goto create_child;
+      break;
+
+      /* This tries to look up the member type we're adding, and goes wrong
+	 if the struct is in the child and the member type is in the parent.  */
+    case ADD_MEMBER_ENCODED:
+      if ((stype = ctf_add_struct (cfp, CTF_ADD_ROOT, "foo")) == CTF_ERR)
+	goto create_child;
+
+      if (ctf_add_member_encoded (cfp, stype, "cmember", ptype, 5, e) == CTF_ERR)
+	goto create_child;
+      break;
+
+      /* This tries to look up the array we're resetting the state of.  */
+    case SET_ARRAY:
+      {
+	ctf_arinfo_t ar;
+
+	ar.ctr_contents = ptype;
+	ar.ctr_index = ptype;
+	ar.ctr_nelems = 5;
+
+	if ((stype = ctf_add_array (pfp, CTF_ADD_ROOT, &ar)) == CTF_ERR)
+	  goto create_child;
+
+	if (ctf_set_array (cfp, stype, &ar) == CTF_ERR)
+	  goto create_child;
+	break;
+      }
+    }
+
+  ctf_dict_close (cfp);
+  ctf_dict_close (pfp);
+
+  return;
+
+ create_err:
+  fprintf (stderr, "Creation failed: %s\n", ctf_errmsg (err));
+  exit (1);
+ create_parent:
+  fprintf (stderr, "Cannot create parent type: %s\n", ctf_errmsg (ctf_errno (pfp)));
+  exit (1);
+ create_child:
+  fprintf (stderr, "Cannot create child type: %s\n", ctf_errmsg (ctf_errno (cfp)));
+  exit (1);
+}