Re: [PATCH bpf-next v1 02/14] resolve_btfids: Fix memory leaks reported by ASAN

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Feb 11, 2026 at 05:13:44PM -0800, Ihor Solodrai wrote:
> Running resolve_btfids with ASAN reveals memory leaks in btf_id
> handling.
> 
> - Change get_id() to use a local buffer
> - Make btf_id__add() strdup the name internally
> - Add btf_id__free_all() that frees all nodese of a tree
> - Call the cleanup function on exit for every tree
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

Acked-by: Jiri Olsa <jolsa@kernel.org>

one nit below

thanks,
jirka


> ---
>  tools/bpf/resolve_btfids/main.c | 78 +++++++++++++++++++++------------
>  1 file changed, 51 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index ca7fcd03efb6..863fa0faaa31 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -226,7 +226,7 @@ static struct btf_id *btf_id__find(struct rb_root *root, const char *name)
>  }
>  
>  static struct btf_id *__btf_id__add(struct rb_root *root,
> -				    char *name,
> +				    const char *name,
>  				    enum btf_id_kind kind,
>  				    bool unique)
>  {
> @@ -250,7 +250,11 @@ static struct btf_id *__btf_id__add(struct rb_root *root,
>  	id = zalloc(sizeof(*id));
>  	if (id) {
>  		pr_debug("adding symbol %s\n", name);
> -		id->name = name;
> +		id->name = strdup(name);
> +		if (!id->name) {
> +			free(id);
> +			return NULL;
> +		}
>  		id->kind = kind;
>  		rb_link_node(&id->rb_node, parent, p);
>  		rb_insert_color(&id->rb_node, root);
> @@ -258,17 +262,17 @@ static struct btf_id *__btf_id__add(struct rb_root *root,
>  	return id;
>  }
>  
> -static inline struct btf_id *btf_id__add(struct rb_root *root, char *name, enum btf_id_kind kind)
> +static inline struct btf_id *btf_id__add(struct rb_root *root, const char *name, enum btf_id_kind kind)
>  {
>  	return __btf_id__add(root, name, kind, false);
>  }
>  
> -static inline struct btf_id *btf_id__add_unique(struct rb_root *root, char *name, enum btf_id_kind kind)
> +static inline struct btf_id *btf_id__add_unique(struct rb_root *root, const char *name, enum btf_id_kind kind)
>  {
>  	return __btf_id__add(root, name, kind, true);
>  }
>  
> -static char *get_id(const char *prefix_end)
> +static int get_id(const char *prefix_end, char *buf, size_t buf_sz)
>  {
>  	/*
>  	 * __BTF_ID__func__vfs_truncate__0
> @@ -277,28 +281,28 @@ static char *get_id(const char *prefix_end)
>  	 */
>  	int len = strlen(prefix_end);
>  	int pos = sizeof("__") - 1;
> -	char *p, *id;
> +	char *p;
>  
>  	if (pos >= len)
> -		return NULL;
> +		return -1;
>  
> -	id = strdup(prefix_end + pos);
> -	if (id) {
> -		/*
> -		 * __BTF_ID__func__vfs_truncate__0
> -		 * id =            ^
> -		 *
> -		 * cut the unique id part
> -		 */
> -		p = strrchr(id, '_');
> -		p--;
> -		if (*p != '_') {
> -			free(id);
> -			return NULL;
> -		}
> -		*p = '\0';
> -	}
> -	return id;
> +	if (len - pos >= buf_sz)
> +		return -1;
> +
> +	strcpy(buf, prefix_end + pos);
> +	/*
> +	 * __BTF_ID__func__vfs_truncate__0
> +	 * buf =           ^
> +	 *
> +	 * cut the unique id part
> +	 */
> +	p = strrchr(buf, '_');
> +	p--;
> +	if (*p != '_')
> +		return -1;
> +	*p = '\0';
> +
> +	return 0;
>  }
>  
>  static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind kind)
> @@ -335,10 +339,9 @@ static struct btf_id *add_set(struct object *obj, char *name, enum btf_id_kind k
>  
>  static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
>  {
> -	char *id;
> +	char id[KSYM_NAME_LEN];
>  
> -	id = get_id(name + size);
> -	if (!id) {
> +	if (get_id(name + size, id, sizeof(id))) {
>  		pr_err("FAILED to parse symbol name: %s\n", name);
>  		return NULL;
>  	}
> @@ -346,6 +349,22 @@ static struct btf_id *add_symbol(struct rb_root *root, char *name, size_t size)
>  	return btf_id__add(root, id, BTF_ID_KIND_SYM);
>  }
>  
> +static void btf_id__free_all(struct rb_root *root)
> +{
> +	struct rb_node *node, *next;
> +	struct btf_id *id;
> +
> +	node = rb_first(root);
> +	while (node) {
> +		next = rb_next(node);
> +		id = rb_entry(node, struct btf_id, rb_node);
> +		rb_erase(node, root);
> +		free(id->name);
> +		free(id);
> +		node = next;
> +	}
> +}


nit, I think we could do it with just single rb_node pointer, like:

        next = rb_first(root);
        while (next) {
                id = rb_entry(next, struct btf_id, rb_node);
                next = rb_next(&id->rb_node);
                rb_erase(&id->rb_node, root);
                free(id->name);
                free(id);
        }

jirka

> +
>  static void bswap_32_data(void *data, u32 nr_bytes)
>  {
>  	u32 cnt, i;
> @@ -1547,6 +1566,11 @@ int main(int argc, const char **argv)
>  out:
>  	btf__free(obj.base_btf);
>  	btf__free(obj.btf);
> +	btf_id__free_all(&obj.structs);
> +	btf_id__free_all(&obj.unions);
> +	btf_id__free_all(&obj.typedefs);
> +	btf_id__free_all(&obj.funcs);
> +	btf_id__free_all(&obj.sets);
>  	if (obj.efile.elf) {
>  		elf_end(obj.efile.elf);
>  		close(obj.efile.fd);
> -- 
> 2.53.0
> 
>