diff mbox series

[05/11] locate: Fix UB on memcpy call

Message ID 20250507142110.3452012-6-adhemerval.zanella@linaro.org
State New
Headers show
Series Add initial support for --enable-ubsan | expand

Commit Message

Adhemerval Zanella May 7, 2025, 2:17 p.m. UTC 
The ubsan triggers:

UBSAN: Undefined behaviour in programs/charmap.c:908:2 null pointer passed as argument 2, nonnull attribute declared at unknown:0:0

This is not an isseu since size is always '0' in this case.
---
 locale/programs/charmap.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Collin Funk May 8, 2025, 3 a.m. UTC | #1 
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> The ubsan triggers:
>
> UBSAN: Undefined behaviour in programs/charmap.c:908:2 null pointer passed as argument 2, nonnull attribute declared at unknown:0:0
>
> This is not an isseu since size is always '0' in this case.

s/isseu/issue :)

> -
> -      memcpy (new_rules, result->width_rules,
> -	      result->nwidth_rules_max * sizeof (struct width_rule));
> +      if (result->width_rules != NULL)
> +	memcpy (new_rules, result->width_rules,
> +		result->nwidth_rules_max * sizeof (struct width_rule));
[...]

This should be well defined in the future as long as the size is always
zero [1].

But I guess since it is still undefined, your change is correct...

Collin

[1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf
Adhemerval Zanella May 28, 2025, 7:22 p.m. UTC | #2 
On 08/05/25 00:00, Collin Funk wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> 
>> The ubsan triggers:
>>
>> UBSAN: Undefined behaviour in programs/charmap.c:908:2 null pointer passed as argument 2, nonnull attribute declared at unknown:0:0
>>
>> This is not an isseu since size is always '0' in this case.
> 
> s/isseu/issue :)
> 
>> -
>> -      memcpy (new_rules, result->width_rules,
>> -	      result->nwidth_rules_max * sizeof (struct width_rule));
>> +      if (result->width_rules != NULL)
>> +	memcpy (new_rules, result->width_rules,
>> +		result->nwidth_rules_max * sizeof (struct width_rule));
> [...]
> 
> This should be well defined in the future as long as the size is always
> zero [1].
> 
> But I guess since it is still undefined, your change is correct...

I think we might change it once we change the internal standard used
to build glibc itself (gnu11). Assuming that -fsanitize=undefined 
does handle it.

> 
> Collin
> 
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3322.pdf
diff mbox series

Patch

diff --git a/locale/programs/charmap.c b/locale/programs/charmap.c
index 58433c8c5b..07d406e12f 100644
--- a/locale/programs/charmap.c
+++ b/locale/programs/charmap.c
@@ -904,9 +904,9 @@  number of bytes for byte sequence of beginning and end of range not the same: %d
 	(struct width_rule *) obstack_alloc (&result->mem_pool,
 					     (new_size
 					      * sizeof (struct width_rule)));
-
-      memcpy (new_rules, result->width_rules,
-	      result->nwidth_rules_max * sizeof (struct width_rule));
+      if (result->width_rules != NULL)
+	memcpy (new_rules, result->width_rules,
+		result->nwidth_rules_max * sizeof (struct width_rule));
 
       result->width_rules = new_rules;
       result->nwidth_rules_max = new_size;