[aarch64] Allocate enough space for err_str in aarch64_handle_attr_branch_protection
diff mbox series

Message ID HE1PR0802MB2251D65805A4574FE7834AD5E07E0@HE1PR0802MB2251.eurprd08.prod.outlook.com
State New
Headers show
Series
  • [aarch64] Allocate enough space for err_str in aarch64_handle_attr_branch_protection
Related show

Commit Message

Matthew Malcomson Nov. 5, 2019, 11:33 a.m. UTC
-fsanitize=hwaddress found a one-byte overwrite when running the
testsuite here.  aarch64_handle_attr_branch_protection allocates
`strlen(str)` bytes for an error string, which is populated by
`strcpy(..., str)` in the case where the branch protection string is
completely invalid.

Tested on aarch64 with hwasan (though not a full bootstrap since it's
obvious).

gcc/ChangeLog:

2019-11-05  Matthew Malcomson  <matthew.malcomson@arm.com>

	* config/aarch64/aarch64.c (aarch64_handle_attr_cpu): Allocate
	enough bytes for the NULL character.



###############     Attachment also inlined for ease of reply    ###############
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 232317d4a5a4a16529f573eef5a8d7a068068207..fc03faa8f8d459a84024d4394fff375b72d31264 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13298,7 +13298,7 @@ aarch64_handle_attr_cpu (const char *str)
  static bool
  aarch64_handle_attr_branch_protection (const char* str)
  {
-  char *err_str = (char *) xmalloc (strlen (str));
+  char *err_str = (char *) xmalloc (strlen (str) + 1);
   enum aarch64_parse_opt_result res = aarch64_parse_branch_protection (str,
 								      &err_str);
   bool success = false;

Comments

Kyrylo Tkachov Nov. 5, 2019, 11:38 a.m. UTC | #1
Hi Matthew,

On 11/5/19 11:33 AM, Matthew Malcomson wrote:
> -fsanitize=hwaddress found a one-byte overwrite when running the
> testsuite here.  aarch64_handle_attr_branch_protection allocates
> `strlen(str)` bytes for an error string, which is populated by
> `strcpy(..., str)` in the case where the branch protection string is
> completely invalid.
>
> Tested on aarch64 with hwasan (though not a full bootstrap since it's
> obvious).
>
Nice to see hwasan catching these things!

Ok.

Thanks,

Kyrill



> gcc/ChangeLog:
>
> 2019-11-05  Matthew Malcomson <matthew.malcomson@arm.com>
>
>         * config/aarch64/aarch64.c (aarch64_handle_attr_cpu): Allocate
>         enough bytes for the NULL character.
>
>
>
> ###############     Attachment also inlined for ease of reply    
> ###############
>
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 
> 232317d4a5a4a16529f573eef5a8d7a068068207..fc03faa8f8d459a84024d4394fff375b72d31264 
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -13298,7 +13298,7 @@ aarch64_handle_attr_cpu (const char *str)
>   static bool
>   aarch64_handle_attr_branch_protection (const char* str)
>   {
> -  char *err_str = (char *) xmalloc (strlen (str));
> +  char *err_str = (char *) xmalloc (strlen (str) + 1);
>    enum aarch64_parse_opt_result res = aarch64_parse_branch_protection 
> (str,
> &err_str);
>    bool success = false;
>

Patch
diff mbox series

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 232317d4a5a4a16529f573eef5a8d7a068068207..fc03faa8f8d459a84024d4394fff375b72d31264 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -13298,7 +13298,7 @@  aarch64_handle_attr_cpu (const char *str)
  static bool
  aarch64_handle_attr_branch_protection (const char* str)
  {
-  char *err_str = (char *) xmalloc (strlen (str));
+  char *err_str = (char *) xmalloc (strlen (str) + 1);
   enum aarch64_parse_opt_result res = aarch64_parse_branch_protection (str,
 								      &err_str);
   bool success = false;