[PATCH dwarves v2] btf_encoder: fix memory access bugs

[PATCH dwarves v2] btf_encoder: fix memory access bugs

[Ihor Solodrai]

When compiled with address sanitizer, a couple of errors were reported
on pahole BTF encoding:
  * A memory leak of strdup(func->alias), due to unchecked
    reassignment.
  * A read of uninitialized memory in gobuffer__sort or bsearch in
    case btf_funcs gobuffer is empty.

Used compiler flags:
    -fsanitize=undefined,address
    -fsanitize-recover=address
    -fno-omit-frame-pointer

v1: https://lore.kernel.org/dwarves/20241213233205.633927-1-ihor.solodrai@pm.me/

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
---
 btf_encoder.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 3754884..fbc9509 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -1794,7 +1794,8 @@ static int btf_encoder__collect_btf_funcs(struct btf_encoder *encoder, struct go
 	}
 
 	/* Now that we've collected funcs, sort them by name */
-	gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
+	if (gobuffer__nr_entries(funcs) > 0)
+		gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
 
 	err = 0;
 out:
@@ -1954,6 +1955,11 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
 		goto out;
 	}
 
+	if (gobuffer__nr_entries(&btf_funcs) == 0) {
+		err = 0;
+		goto out;
+	}
+
 	/* First collect all kfunc set ranges.
 	 *
 	 * Note we choose not to sort these ranges and accept a linear
@@ -2607,7 +2613,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
 						       ", has optimized-out parameters" :
 						       fn->proto.unexpected_reg ? ", has unexpected register use by params" :
 						       "");
-					func->alias = strdup(name);
+					if (!func->alias)
+						func->alias = strdup(name);
 				}
 			}
 		} else {
-- 
2.47.1

Re: [PATCH dwarves v2] btf_encoder: fix memory access bugs

[ihor.solodrai]

December 16, 2024 at 10:31 AM, "Ihor Solodrai" <ihor.solodrai@pm.me> wrote:

> 
> When compiled with address sanitizer, a couple of errors were reported
> 
> on pahole BTF encoding:
> 
>  * A memory leak of strdup(func->alias), due to unchecked
> 
>  reassignment.
> 
>  * A read of uninitialized memory in gobuffer__sort or bsearch in
> 
>  case btf_funcs gobuffer is empty.
> 
> Used compiler flags:
> 
>  -fsanitize=undefined,address
> 
>  -fsanitize-recover=address
> 
>  -fno-omit-frame-pointer
> 
> v1: https://lore.kernel.org/dwarves/20241213233205.633927-1-ihor.solodrai@pm.me/
> 
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
> 
> ---
> 
>  btf_encoder.c | 11 +++++++++--
> 
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Alan, Arnaldo,

This patch hasn't been applied.
Just a reminder in case it fell off the radar.

Thanks.

Re: [PATCH dwarves v2] btf_encoder: fix memory access bugs

[Alan Maguire]

On 28/01/2025 00:03, ihor.solodrai@linux.dev wrote:
> December 16, 2024 at 10:31 AM, "Ihor Solodrai" <ihor.solodrai@pm.me> wrote:
> 
>>
>> When compiled with address sanitizer, a couple of errors were reported
>>
>> on pahole BTF encoding:
>>
>>  * A memory leak of strdup(func->alias), due to unchecked
>>
>>  reassignment.
>>
>>  * A read of uninitialized memory in gobuffer__sort or bsearch in
>>
>>  case btf_funcs gobuffer is empty.
>>
>> Used compiler flags:
>>
>>  -fsanitize=undefined,address
>>
>>  -fsanitize-recover=address
>>
>>  -fno-omit-frame-pointer
>>
>> v1: https://lore.kernel.org/dwarves/20241213233205.633927-1-ihor.solodrai@pm.me/
>>
>> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
>>
>> ---
>>
>>  btf_encoder.c | 11 +++++++++--
>>
>>  1 file changed, 9 insertions(+), 2 deletions(-)
>>
> 
> Alan, Arnaldo,
> 
> This patch hasn't been applied.
> Just a reminder in case it fell off the radar.
>

Thanks for the reminder; we'll make sure this one gets applied shortly.
I wonder if we should add the -fsanitize flags to CFLAGS for RELEASE mode?

Re: [PATCH dwarves v2] btf_encoder: fix memory access bugs

[Ihor Solodrai]

On 1/31/25 5:14 AM, Alan Maguire wrote:
 >> [...]
 >
 > Thanks for the reminder; we'll make sure this one gets applied shortly.
 > I wonder if we should add the -fsanitize flags to CFLAGS for RELEASE 
mode?
 >

The issue with these flags is that they significantly reduce
performance. We might consider adding them for debug builds, but
certainly not for the release.

Re: [PATCH dwarves v2] btf_encoder: fix memory access bugs

[Alan Maguire]

On 16/12/2024 18:31, Ihor Solodrai wrote:
> When compiled with address sanitizer, a couple of errors were reported
> on pahole BTF encoding:
>   * A memory leak of strdup(func->alias), due to unchecked
>     reassignment.
>   * A read of uninitialized memory in gobuffer__sort or bsearch in
>     case btf_funcs gobuffer is empty.
> 
> Used compiler flags:
>     -fsanitize=undefined,address
>     -fsanitize-recover=address
>     -fno-omit-frame-pointer
> 
> v1: https://lore.kernel.org/dwarves/20241213233205.633927-1-ihor.solodrai@pm.me/
> 
> Reviewed-by: Alan Maguire <alan.maguire@oracle.com>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>

applied to the next branch of

https://git.kernel.org/pub/scm/devel/pahole/pahole.git/

thanks!

> ---
>  btf_encoder.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 3754884..fbc9509 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c
> @@ -1794,7 +1794,8 @@ static int btf_encoder__collect_btf_funcs(struct btf_encoder *encoder, struct go
>  	}
>  
>  	/* Now that we've collected funcs, sort them by name */
> -	gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
> +	if (gobuffer__nr_entries(funcs) > 0)
> +		gobuffer__sort(funcs, sizeof(struct btf_func), btf_func_cmp);
>  
>  	err = 0;
>  out:
> @@ -1954,6 +1955,11 @@ static int btf_encoder__tag_kfuncs(struct btf_encoder *encoder)
>  		goto out;
>  	}
>  
> +	if (gobuffer__nr_entries(&btf_funcs) == 0) {
> +		err = 0;
> +		goto out;
> +	}
> +
>  	/* First collect all kfunc set ranges.
>  	 *
>  	 * Note we choose not to sort these ranges and accept a linear
> @@ -2607,7 +2613,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co
>  						       ", has optimized-out parameters" :
>  						       fn->proto.unexpected_reg ? ", has unexpected register use by params" :
>  						       "");
> -					func->alias = strdup(name);
> +					if (!func->alias)
> +						func->alias = strdup(name);
>  				}
>  			}
>  		} else {