WIP MachO binutils port assisted by AI I may consider upstreaming this in the future.
0

Configure Feed

Select the types of activity you want to include in your feed.

libctf: do not corrupt strings across ctf_serialize

The preceding change revealed a new bug: the string table is sorted for
better compression, so repeated serialization with type (or member)
additions in the middle can move strings around. But every
serialization flushes the set of refs (the memory locations that are
automatically updated with a final string offset when the strtab is
updated), so if we are not to have string offsets go stale, we must do
all ref additions within the serialization code (which walks the
complete set of types and symbols anyway). Unfortunately, we were adding
one ref in another place: the type name in the dynamic type definitions,
which has a ref added to it by ctf_add_generic.

So adding a type, serializing (via, say, one of the ctf_write
functions), adding another type with a name that sorts earlier, and
serializing again will corrupt the name of the first type because it no
longer had a ref pointing to its dtd entry's name when its string offset
was shifted later in the strtab to mae way for the other type.

To ensure that we don't miss strings, we also maintain a set of *pending
refs* that will be added later (during serialization), and remove
entries from that set when the ref is finally added. We always use
ctf_str_add_pending outside ctf-serialize.c, ensure that ctf_serialize
adds all strtab offsets as refs (even those in the dtds) on every
serialization, and mandate that no refs are live on entry to
ctf_serialize and that all pending refs are gone before strtab
finalization. (Of necessity ctf_serialize has to traverse all strtab
offsets in the dtds in order to serialize them, so adding them as refs
at the same time is easy.)

(Note that we still can't erase unused atoms when we roll back, though
we can erase unused refs: members and enums are still not removed by
rollbacks and might reference strings added after the snapshot.)

libctf/ChangeLog
2021-03-18 Nick Alcock <nick.alcock@oracle.com>

* ctf-hash.c (ctf_dynset_elements): New.
* ctf-impl.h (ctf_dynset_elements): Declare it.
(ctf_str_add_pending): Likewise.
(ctf_dict_t) <ctf_str_pending_ref>: New, set of refs that must be
added during serialization.
* ctf-string.c (ctf_str_create_atoms): Initialize it.
(CTF_STR_ADD_REF): New flag.
(CTF_STR_MAKE_PROVISIONAL): Likewise.
(CTF_STR_PENDING_REF): Likewise.
(ctf_str_add_ref_internal): Take a flags word rather than int
params. Populate, and clear out, ctf_str_pending_ref.
(ctf_str_add): Adjust accordingly.
(ctf_str_add_external): Likewise.
(ctf_str_add_pending): New.
(ctf_str_remove_ref): Also remove the potential ref if it is a
pending ref.
* ctf-serialize.c (ctf_serialize): Prohibit addition of strings
with ctf_str_add_ref before serialization. Ensure that the
ctf_str_pending_ref set is empty before strtab finalization.
(ctf_emit_type_sect): Add a ref to the ctt_name.
* ctf-create.c (ctf_add_generic): Add the ctt_name as a pending
ref.
* testsuite/libctf-writable/reserialize-strtab-corruption.*: New test.


Nick Alcock (Mar 18, 2021, 12:40 PM UTC) 986e9e3a 2a05d50e

+209 -12
+26
libctf/ChangeLog
··· 1 1 2021-03-18 Nick Alcock <nick.alcock@oracle.com> 2 2 3 + * ctf-hash.c (ctf_dynset_elements): New. 4 + * ctf-impl.h (ctf_dynset_elements): Declare it. 5 + (ctf_str_add_pending): Likewise. 6 + (ctf_dict_t) <ctf_str_pending_ref>: New, set of refs that must be 7 + added during serialization. 8 + * ctf-string.c (ctf_str_create_atoms): Initialize it. 9 + (CTF_STR_ADD_REF): New flag. 10 + (CTF_STR_MAKE_PROVISIONAL): Likewise. 11 + (CTF_STR_PENDING_REF): Likewise. 12 + (ctf_str_add_ref_internal): Take a flags word rather than int 13 + params. Populate, and clear out, ctf_str_pending_ref. 14 + (ctf_str_add): Adjust accordingly. 15 + (ctf_str_add_external): Likewise. 16 + (ctf_str_add_pending): New. 17 + (ctf_str_remove_ref): Also remove the potential ref if it is a 18 + pending ref. 19 + * ctf-serialize.c (ctf_serialize): Prohibit addition of strings 20 + with ctf_str_add_ref before serialization. Ensure that the 21 + ctf_str_pending_ref set is empty before strtab finalization. 22 + (ctf_emit_type_sect): Add a ref to the ctt_name. 23 + * ctf-create.c (ctf_add_generic): Add the ctt_name as a pending 24 + ref. 25 + * testsuite/libctf-writable/reserialize-strtab-corruption.*: New test. 26 + 27 + 2021-03-18 Nick Alcock <nick.alcock@oracle.com> 28 + 3 29 * ctf-serialize.c (ctf_serialize): Preserve ctf_typemax across 4 30 serialization. 5 31
+2 -1
libctf/ctf-create.c
··· 439 439 type = ++fp->ctf_typemax; 440 440 type = LCTF_INDEX_TO_TYPE (fp, type, (fp->ctf_flags & LCTF_CHILD)); 441 441 442 - dtd->dtd_data.ctt_name = ctf_str_add_ref (fp, name, &dtd->dtd_data.ctt_name); 442 + dtd->dtd_data.ctt_name = ctf_str_add_pending (fp, name, 443 + &dtd->dtd_data.ctt_name); 443 444 dtd->dtd_type = type; 444 445 445 446 if (dtd->dtd_data.ctt_name == 0 && name != NULL && name[0] != '\0')
+6
libctf/ctf-hash.c
··· 672 672 return NULL; 673 673 } 674 674 675 + size_t 676 + ctf_dynset_elements (ctf_dynset_t *hp) 677 + { 678 + return htab_elements ((struct htab *) hp); 679 + } 680 + 675 681 /* TRUE/FALSE return. */ 676 682 int 677 683 ctf_dynset_exists (ctf_dynset_t *hp, const void *key, const void **orig_key)
+4 -1
libctf/ctf-impl.h
··· 398 398 ctf_names_t ctf_names; /* Hash table of remaining type names. */ 399 399 ctf_lookup_t ctf_lookups[5]; /* Pointers to nametabs for name lookup. */ 400 400 ctf_strs_t ctf_str[2]; /* Array of string table base and bounds. */ 401 - ctf_dynhash_t *ctf_str_atoms; /* Hash table of ctf_str_atoms_t. */ 401 + ctf_dynhash_t *ctf_str_atoms; /* Hash table of ctf_str_atoms_t. */ 402 + ctf_dynset_t *ctf_str_pending_ref; /* Locations awaiting ref addition. */ 402 403 uint64_t ctf_str_num_refs; /* Number of refs to cts_str_atoms. */ 403 404 uint32_t ctf_str_prov_offset; /* Latest provisional offset assigned so far. */ 404 405 unsigned char *ctf_base; /* CTF file pointer. */ ··· 673 674 extern void ctf_dynset_remove (ctf_dynset_t *, const void *); 674 675 extern void ctf_dynset_destroy (ctf_dynset_t *); 675 676 extern void *ctf_dynset_lookup (ctf_dynset_t *, const void *); 677 + extern size_t ctf_dynset_elements (ctf_dynset_t *); 676 678 extern int ctf_dynset_exists (ctf_dynset_t *, const void *key, 677 679 const void **orig_key); 678 680 extern int ctf_dynset_next (ctf_dynset_t *, ctf_next_t **, void **key); ··· 731 733 extern void ctf_str_free_atoms (ctf_dict_t *); 732 734 extern uint32_t ctf_str_add (ctf_dict_t *, const char *); 733 735 extern uint32_t ctf_str_add_ref (ctf_dict_t *, const char *, uint32_t *ref); 736 + extern uint32_t ctf_str_add_pending (ctf_dict_t *, const char *, uint32_t *); 734 737 extern int ctf_str_add_external (ctf_dict_t *, const char *, uint32_t offset); 735 738 extern void ctf_str_remove_ref (ctf_dict_t *, const char *, uint32_t *ref); 736 739 extern void ctf_str_rollback (ctf_dict_t *, ctf_snapshot_id_t);
+23 -1
libctf/ctf-serialize.c
··· 870 870 copied = (ctf_stype_t *) t; /* name is at the start: constant offset. */ 871 871 if (copied->ctt_name 872 872 && (name = ctf_strraw (fp, copied->ctt_name)) != NULL) 873 - ctf_str_add_ref (fp, name, &copied->ctt_name); 873 + { 874 + ctf_str_add_ref (fp, name, &copied->ctt_name); 875 + ctf_str_add_ref (fp, name, &dtd->dtd_data.ctt_name); 876 + } 874 877 t += len; 875 878 876 879 switch (kind) ··· 955 958 ctf_varent_t *dvarents; 956 959 ctf_strs_writable_t strtab; 957 960 int err; 961 + int num_missed_str_refs; 958 962 959 963 unsigned char *t; 960 964 unsigned long i; ··· 972 976 /* Update required? */ 973 977 if (!(fp->ctf_flags & LCTF_DIRTY)) 974 978 return 0; 979 + 980 + /* The strtab refs table must be empty at this stage. Any refs already added 981 + will be corrupted by any modifications, including reserialization, after 982 + strtab finalization is complete. Only this function, and functions it 983 + calls, may add refs, and all memory locations (including in the dtds) 984 + containing strtab offsets must be traversed as part of serialization, and 985 + refs added. */ 986 + 987 + if (!ctf_assert (fp, fp->ctf_str_num_refs == 0)) 988 + return -1; /* errno is set for us. */ 975 989 976 990 /* Fill in an initial CTF header. We will leave the label, object, 977 991 and function sections empty and only output a header, type section, ··· 1051 1065 ctf_emit_type_sect (fp, &t); 1052 1066 1053 1067 assert (t == (unsigned char *) buf + sizeof (ctf_header_t) + hdr.cth_stroff); 1068 + 1069 + /* Every string added outside serialization by ctf_str_add_pending should 1070 + now have been added by ctf_add_ref. */ 1071 + num_missed_str_refs = ctf_dynset_elements (fp->ctf_str_pending_ref); 1072 + if (!ctf_assert (fp, num_missed_str_refs == 0)) 1073 + goto err; /* errno is set for us. */ 1054 1074 1055 1075 /* Construct the final string table and fill out all the string refs with the 1056 1076 final offsets. Then purge the refs list, because we're about to move this ··· 1154 1174 ctf_str_free_atoms (nfp); 1155 1175 nfp->ctf_str_atoms = fp->ctf_str_atoms; 1156 1176 nfp->ctf_prov_strtab = fp->ctf_prov_strtab; 1177 + nfp->ctf_str_pending_ref = fp->ctf_str_pending_ref; 1157 1178 fp->ctf_str_atoms = NULL; 1158 1179 fp->ctf_prov_strtab = NULL; 1180 + fp->ctf_str_pending_ref = NULL; 1159 1181 memset (&fp->ctf_dtdefs, 0, sizeof (ctf_list_t)); 1160 1182 memset (&fp->ctf_errs_warnings, 0, sizeof (ctf_list_t)); 1161 1183 fp->ctf_add_processing = NULL;
+52 -9
libctf/ctf-string.c
··· 103 103 { 104 104 fp->ctf_str_atoms = ctf_dynhash_create (ctf_hash_string, ctf_hash_eq_string, 105 105 free, ctf_str_free_atom); 106 - if (fp->ctf_str_atoms == NULL) 106 + if (!fp->ctf_str_atoms) 107 107 return -ENOMEM; 108 108 109 109 if (!fp->ctf_prov_strtab) ··· 112 112 NULL, NULL); 113 113 if (!fp->ctf_prov_strtab) 114 114 goto oom_prov_strtab; 115 + 116 + if (!fp->ctf_str_pending_ref) 117 + fp->ctf_str_pending_ref = ctf_dynset_create (htab_hash_pointer, 118 + htab_eq_pointer, 119 + NULL); 120 + if (!fp->ctf_str_pending_ref) 121 + goto oom_str_pending_ref; 115 122 116 123 errno = 0; 117 124 ctf_str_add (fp, ""); ··· 123 130 oom_str_add: 124 131 ctf_dynhash_destroy (fp->ctf_prov_strtab); 125 132 fp->ctf_prov_strtab = NULL; 133 + oom_str_pending_ref: 134 + ctf_dynset_destroy (fp->ctf_str_pending_ref); 135 + fp->ctf_str_pending_ref = NULL; 126 136 oom_prov_strtab: 127 137 ctf_dynhash_destroy (fp->ctf_str_atoms); 128 138 fp->ctf_str_atoms = NULL; ··· 135 145 { 136 146 ctf_dynhash_destroy (fp->ctf_prov_strtab); 137 147 ctf_dynhash_destroy (fp->ctf_str_atoms); 148 + ctf_dynset_destroy (fp->ctf_str_pending_ref); 138 149 } 150 + 151 + #define CTF_STR_ADD_REF 0x1 152 + #define CTF_STR_MAKE_PROVISIONAL 0x2 153 + #define CTF_STR_PENDING_REF 0x4 139 154 140 155 /* Add a string to the atoms table, copying the passed-in string. Return the 141 156 atom added. Return NULL only when out of memory (and do not touch the ··· 144 159 provisional strtab. */ 145 160 static ctf_str_atom_t * 146 161 ctf_str_add_ref_internal (ctf_dict_t *fp, const char *str, 147 - int add_ref, int make_provisional, uint32_t *ref) 162 + int flags, uint32_t *ref) 148 163 { 149 164 char *newstr = NULL; 150 165 ctf_str_atom_t *atom = NULL; ··· 152 167 153 168 atom = ctf_dynhash_lookup (fp->ctf_str_atoms, str); 154 169 155 - if (add_ref) 170 + if (flags & CTF_STR_ADD_REF) 156 171 { 157 172 if ((aref = malloc (sizeof (struct ctf_str_atom_ref))) == NULL) 158 173 return NULL; ··· 161 176 162 177 if (atom) 163 178 { 164 - if (add_ref) 179 + if (flags & CTF_STR_ADD_REF) 165 180 { 181 + ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref); 166 182 ctf_list_append (&atom->csa_refs, aref); 167 183 fp->ctf_str_num_refs++; 168 184 } ··· 182 198 atom->csa_str = newstr; 183 199 atom->csa_snapshot_id = fp->ctf_snapshots; 184 200 185 - if (make_provisional) 201 + if (flags & CTF_STR_MAKE_PROVISIONAL) 186 202 { 187 203 atom->csa_offset = fp->ctf_str_prov_offset; 188 204 ··· 193 209 fp->ctf_str_prov_offset += strlen (atom->csa_str) + 1; 194 210 } 195 211 196 - if (add_ref) 212 + if (flags & CTF_STR_PENDING_REF) 197 213 { 214 + if (ctf_dynset_insert (fp->ctf_str_pending_ref, (void *) ref) < 0) 215 + goto oom; 216 + } 217 + else if (flags & CTF_STR_ADD_REF) 218 + { 219 + ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref); 198 220 ctf_list_append (&atom->csa_refs, aref); 199 221 fp->ctf_str_num_refs++; 200 222 } ··· 222 244 if (!str) 223 245 str = ""; 224 246 225 - atom = ctf_str_add_ref_internal (fp, str, FALSE, TRUE, 0); 247 + atom = ctf_str_add_ref_internal (fp, str, CTF_STR_MAKE_PROVISIONAL, 0); 226 248 if (!atom) 227 249 return 0; 228 250 ··· 240 262 if (!str) 241 263 str = ""; 242 264 243 - atom = ctf_str_add_ref_internal (fp, str, TRUE, TRUE, ref); 265 + atom = ctf_str_add_ref_internal (fp, str, CTF_STR_ADD_REF 266 + | CTF_STR_MAKE_PROVISIONAL, ref); 267 + if (!atom) 268 + return 0; 269 + 270 + return atom->csa_offset; 271 + } 272 + 273 + /* Like ctf_str_add_ref(), but notes that this memory location must be added as 274 + a ref by a later serialization phase, rather than adding it itself. */ 275 + uint32_t 276 + ctf_str_add_pending (ctf_dict_t *fp, const char *str, uint32_t *ref) 277 + { 278 + ctf_str_atom_t *atom; 279 + 280 + if (!str) 281 + str = ""; 282 + 283 + atom = ctf_str_add_ref_internal (fp, str, CTF_STR_PENDING_REF 284 + | CTF_STR_MAKE_PROVISIONAL, ref); 244 285 if (!atom) 245 286 return 0; 246 287 ··· 257 298 if (!str) 258 299 str = ""; 259 300 260 - atom = ctf_str_add_ref_internal (fp, str, FALSE, FALSE, 0); 301 + atom = ctf_str_add_ref_internal (fp, str, 0, 0); 261 302 if (!atom) 262 303 return 0; 263 304 ··· 307 348 free (aref); 308 349 } 309 350 } 351 + 352 + ctf_dynset_remove (fp->ctf_str_pending_ref, (void *) ref); 310 353 } 311 354 312 355 /* A ctf_dynhash_iter_remove() callback that removes atoms later than a given
+91
libctf/testsuite/libctf-writable/reserialize-strtab-corruption.c
··· 1 + /* Make sure serializing a dict (possibly repeatedly) does not corrupt either 2 + type lookup or the string content of the dict. */ 3 + 4 + #include <ctf-api.h> 5 + #include <stdio.h> 6 + #include <stdlib.h> 7 + 8 + int 9 + main (int argc, char *argv[]) 10 + { 11 + ctf_dict_t *fp; 12 + ctf_id_t zygal, autoschediastic; 13 + ctf_snapshot_id_t snap; 14 + unsigned char *foo; 15 + size_t foo_size; 16 + const char *bar; 17 + int err; 18 + char name[64]; 19 + 20 + /* Adding things after serialization should not corrupt names created before 21 + serialization. */ 22 + 23 + if ((fp = ctf_create (&err)) == NULL) 24 + goto create_err; 25 + 26 + if ((zygal = ctf_add_struct (fp, CTF_ADD_ROOT, "zygal")) == CTF_ERR) 27 + goto add_err; 28 + 29 + if ((foo = ctf_write_mem (fp, &foo_size, 4096)) == NULL) 30 + goto write_err; 31 + free (foo); 32 + 33 + if (ctf_type_name (fp, zygal, name, sizeof (name)) == NULL) 34 + fprintf (stderr, "Can't get name of zygal: %s\n", ctf_errmsg (ctf_errno (fp))); 35 + else 36 + printf ("zygal's name is %s\n", name); 37 + 38 + if ((autoschediastic = ctf_add_enum (fp, CTF_ADD_ROOT, "autoschediastic")) == CTF_ERR) 39 + goto add_err; 40 + 41 + if (ctf_type_name (fp, zygal, name, sizeof (name)) == NULL) 42 + fprintf (stderr, "Can't get name of zygal: %s\n", ctf_errmsg (ctf_errno (fp))); 43 + else 44 + printf ("zygal's name is %s\n", name); 45 + 46 + /* Serializing again should not corrupt names either. */ 47 + if ((foo = ctf_write_mem (fp, &foo_size, 4096)) == NULL) 48 + goto write_err; 49 + free (foo); 50 + 51 + if (ctf_type_name (fp, zygal, name, sizeof (name)) == NULL) 52 + fprintf (stderr, "Can't get name of zygal: %s\n", ctf_errmsg (ctf_errno (fp))); 53 + else 54 + printf ("zygal's name is %s\n", name); 55 + 56 + /* Add another new name, roll back, and make sure the strings are 57 + uncorrupted. */ 58 + 59 + snap = ctf_snapshot (fp); 60 + if (ctf_add_enumerator (fp, autoschediastic, "aichmophobia", 0) < 0) 61 + goto add_err; 62 + 63 + if (ctf_rollback (fp, snap) < 0) 64 + goto roll_err; 65 + 66 + if (ctf_type_name (fp, zygal, name, sizeof (name)) == NULL) 67 + fprintf (stderr, "Can't get name of zygal: %s\n", ctf_errmsg (ctf_errno (fp))); 68 + else 69 + printf ("zygal's name is %s after first rollback\n", name); 70 + 71 + if (ctf_type_name (fp, autoschediastic, name, sizeof (name)) == NULL) 72 + fprintf (stderr, "Can't get name of autoschediastic: %s\n", ctf_errmsg (ctf_errno (fp))); 73 + else 74 + printf ("autoschediastic's name is %s after first rollback\n", name); 75 + 76 + ctf_dict_close (fp); 77 + return 0; 78 + 79 + create_err: 80 + fprintf (stderr, "Cannot create: %s\n", ctf_errmsg (err)); 81 + return 1; 82 + add_err: 83 + fprintf (stderr, "Cannot add: %s\n", ctf_errmsg (ctf_errno (fp))); 84 + return 1; 85 + write_err: 86 + fprintf (stderr, "Cannot serialize: %s\n", ctf_errmsg (ctf_errno (fp))); 87 + return 1; 88 + roll_err: 89 + fprintf (stderr, "Cannot roll back: %s\n", ctf_errmsg (ctf_errno (fp))); 90 + return 1; 91 + }
+5
libctf/testsuite/libctf-writable/reserialize-strtab-corruption.lk
··· 1 + zygal's name is struct zygal 2 + zygal's name is struct zygal 3 + zygal's name is struct zygal 4 + zygal's name is struct zygal after first rollback 5 + autoschediastic's name is enum autoschediastic after first rollback