[17/19] libctf: fix ref leak of names of newly-inserted non-root-visible types

Message ID 20240730153707.168357-18-nick.alcock@oracle.com
State New
Headers
Series libctf: various bugfixes (including a write into freed memory), and loosen constraints on enums |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Nick Alcock July 30, 2024, 3:37 p.m. UTC
  A bug in ctf_dtd_delete led to refs in the string table to the
names of non-root-visible types not being removed when the DTD
was.  This seems harmless, but actually it would lead to a write
down a pointer into freed memory if such a type was ctf_rollback()ed
over and then the dict was serialized (updating all the refs as the
strtab was serialized in turn).

Bug introduced in commit fe4c2d55634c700ba527ac4183e05c66e9f93c62
("libctf: create: non-root-visible types should not appear in name tables")
which is included in binutils 2.35.

libctf/
	* ctf-create.c (ctf_dtd_delete): Remove refs for all types
	with names, not just root-visible ones.
---
 libctf/ctf-create.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Comments

Nick Alcock Aug. 2, 2024, 1:16 p.m. UTC | #1
On 30 Jul 2024, Nick Alcock outgrape:

> A bug in ctf_dtd_delete led to refs in the string table to the
> names of non-root-visible types not being removed when the DTD
> was.  This seems harmless, but actually it would lead to a write
> down a pointer into freed memory if such a type was ctf_rollback()ed
> over and then the dict was serialized (updating all the refs as the
> strtab was serialized in turn).
>
> Bug introduced in commit fe4c2d55634c700ba527ac4183e05c66e9f93c62
> ("libctf: create: non-root-visible types should not appear in name tables")
> which is included in binutils 2.35.

... and now this commit is cherry-picked to all the release branches
2.35 -- 2.42 (yes I know this is overkill).

Valgrinded a test run of the lot with a leak-inducing test added to
prove it's really gone (and didn't break anything else).
  

Patch

diff --git a/libctf/ctf-create.c b/libctf/ctf-create.c
index a7544955212..0c8959a997e 100644
--- a/libctf/ctf-create.c
+++ b/libctf/ctf-create.c
@@ -258,10 +258,10 @@  ctf_dtd_delete (ctf_dict_t *fp, ctf_dtdef_t *dtd)
   dtd->dtd_vlen_alloc = 0;
 
   if (dtd->dtd_data.ctt_name
-      && (name = ctf_strraw (fp, dtd->dtd_data.ctt_name)) != NULL
-      && LCTF_INFO_ISROOT (fp, dtd->dtd_data.ctt_info))
+      && (name = ctf_strraw (fp, dtd->dtd_data.ctt_name)) != NULL)
     {
-      ctf_dynhash_remove (ctf_name_table (fp, name_kind), name);
+      if (LCTF_INFO_ISROOT (fp, dtd->dtd_data.ctt_info))
+	ctf_dynhash_remove (ctf_name_table (fp, name_kind), name);
       ctf_str_remove_ref (fp, name, &dtd->dtd_data.ctt_name);
     }