[PATCH] AIX: Fix buffer overflow in fill_fprs

[PATCH] AIX: Fix buffer overflow in fill_fprs

[Chris January]

The callers of fill_fprs expect the first floating point register to be
written to vals[0], but it's currently written to
vals[tdep->ppc_fp0_regnum] which can cause GDB to segfault as the
caller's buffer overflows.

2012-03-08  Chris January  <chris.january@allinea.com>

	* aix-thread.c (fill_sprs): Store the floating point registers at the
correct offsets into vals.
---
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 0a9fae3..0c697f7 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1356,7 +1356,8 @@ fill_fprs (const struct regcache *regcache, double
*vals)
        regno < tdep->ppc_fp0_regnum + ppc_num_fprs;
        regno++)
     if (REG_VALID == regcache_register_status (regcache, regno))
-      regcache_raw_collect (regcache, regno, vals + regno);
+      regcache_raw_collect (regcache, regno,
+			    vals + regno - tdep->ppc_fp0_regnum);
 }
 
 /* Store the special registers into the specified 64-bit and 32-bit

Re: [PATCH] AIX: Fix buffer overflow in fill_fprs

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 849 bytes --]

> The callers of fill_fprs expect the first floating point register to be
> written to vals[0], but it's currently written to
> vals[tdep->ppc_fp0_regnum] which can cause GDB to segfault as the
> caller's buffer overflows.
> 
> 2012-03-08  Chris January  <chris.january@allinea.com>
> 
> 	* aix-thread.c (fill_sprs): Store the floating point registers
> 	at the correct offsets into vals.

This looks right to me. I tested it on AIX for you and checked it in.

While doing this, I looked at the opposite function, and found
that it was ok, but with a regno meaning something different.
The implementation in fill_sprs seems more natural to me, so
I fixed supply_fprs as attached.

Thanks again for the patch.

PS: I reformatted your ChangeLog entry. We try to keep lines under
    70 characters long, with a hard-limit at 80 characters...

-- 
Joel

[-- Attachment #2: 0001-Minor-cleanup-in-aix-thread.c-supply_fprs.patch --]
[-- Type: text/x-diff, Size: 1641 bytes --]

From 7cd4e229d7d23046706970cde3cc34387039a4e7 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 13 Mar 2012 15:21:57 -0700
Subject: [PATCH] Minor cleanup in aix-thread.c:supply_fprs.

This is a minor cleanup that makes supply_fprs more consistent with
how fill_fprs was written.

gdb/ChangeLog:

        * aix-thread.c (supply_fprs): Make more consistent with fill_fprs.
---
 gdb/ChangeLog    |    4 ++++
 gdb/aix-thread.c |    8 +++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4725f54..480ce34 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2012-03-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* aix-thread.c (supply_fprs): Make more consistent with fill_fprs.
+
 2012-03-13  Chris January  <chris.january@allinea.com>
 
 	* aix-thread.c (fill_sprs): Store the floating point registers
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index f9787e3..534f731 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -1075,9 +1075,11 @@ supply_fprs (struct regcache *regcache, double *vals)
      floating-point registers.  */
   gdb_assert (ppc_floating_point_unit_p (gdbarch));
 
-  for (regno = 0; regno < ppc_num_fprs; regno++)
-    regcache_raw_supply (regcache, regno + tdep->ppc_fp0_regnum,
-			 (char *) (vals + regno));
+  for (regno = tdep->ppc_fp0_regnum;
+       regno < tdep->ppc_fp0_regnum + ppc_num_fprs;
+       regno++)
+    regcache_raw_supply (regcache, regno,
+			 (char *) (vals + regno - tdep->ppc_fp0_regnum));
 }
 
 /* Predicate to test whether given register number is a "special" register.  */
-- 
1.7.1