[v2,13/16] RISC-V: avoid buffer underrun in subset parsing

Message ID 4491e1e3-07d5-469f-ac3c-df6e133ba906@suse.com
State New
Headers
Series RISC-V: assorted fixes and (hopefully) improvements |

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

Jan Beulich May 15, 2026, 1:35 p.m. UTC
  In both instances 'p' can be the first character. Inspecting the character
immediately ahead of it is then UB.

To increase similarity between the two checks (each), also move the
increment of q past the check for the (bogus) "<number>p" ending.

Correct indentation in nearby code, where it being wrong is actively
misleading.

Reviewed-by: Jiawei <jiawei@iscas.ac.cn>
---
Why is it that P is recognized as an extension when there's no other
support there for it?
  

Patch

--- a/bfd/elfxx-riscv.c
+++ b/bfd/elfxx-riscv.c
@@ -2113,25 +2113,27 @@  riscv_parse_extensions (riscv_parse_subs
 		find_any_version = true;
 	      else if (find_any_version
 		       && !find_minor_version
+		       && q > subset
 		       && *q == 'p'
 		       && ISDIGIT (*(q - 1)))
-	      find_minor_version = true;
+		find_minor_version = true;
 	      else
 		break;
 	    }
-	  q++;
 
 	  /* Check if the end of extension is 'p' or not.  If yes, then
 	     the second letter from the end cannot be number.  */
-	  if (*(q - 1) == 'p' && ISDIGIT (*(q - 2)))
+	  if (q > subset && *q == 'p' && ISDIGIT (*(q - 1)))
 	    {
-	      *q = '\0';
+	      q[1] = '\0';
 	      rps->error_handler
 		(_("%s: invalid prefixed ISA extension `%s' ends with <number>p"),
 		 arch, subset);
 	      free (subset);
 	      return NULL;
 	    }
+
+	  q++;
 	}
 
       int major_version = RISCV_UNKNOWN_VERSION;
@@ -2672,20 +2674,19 @@  riscv_update_subset1 (riscv_parse_subset
 	    find_any_version = true;
 	  else if (find_any_version
 		   && !find_minor_version
+		   && q > subset
 		   && *q == 'p'
 		   && ISDIGIT (*(q - 1)))
 	    find_minor_version = true;
 	  else
 	    break;
 	}
-      if (len > 0)
-	q++;
 
       /* Check if the end of extension is 'p' or not.  If yes, then
 	 the second letter from the end cannot be number.  */
-      if (len > 1 && *(q - 1) == 'p' && ISDIGIT (*(q - 2)))
+      if (q > subset && *q == 'p' && ISDIGIT (*(q - 1)))
 	{
-	  *q = '\0';
+	  q[1] = '\0';
 	  rps->error_handler
 	    (_("%sinvalid ISA extension `%s' ends with <number>p in %s `%s'"),
 	       errmsg_internal, subset, errmsg_caller, implicit_exts);
@@ -2693,6 +2694,9 @@  riscv_update_subset1 (riscv_parse_subset
 	  return false;
 	}
 
+      if (len > 0)
+	q++;
+
       end_of_version =
 	riscv_parsing_subset_version (q, &major_version, &minor_version);
       *q = '\0';