Bug 24056 - nptl_db: Overrun in _td_store_value and _td_store_value_local.
Summary: nptl_db: Overrun in _td_store_value and _td_store_value_local.
Status: NEW
Alias: None
Product: glibc
Classification: Unclassified
Component: nptl (show other bugs)
Version: 2.30
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-03 17:25 UTC by Carlos O'Donell
Modified: 2019-01-04 07:37 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos O'Donell 2019-01-03 17:25:47 UTC
Two functions _td_store_value and _td_store_value_local take a 'unit32_t desc[2]' instead of using the db_desc_t typedef, and therefore the type they accept is 4-bytes too small (no offset). The callers are using ta_var_* variables which were created with DB_VARIABLE() and so are using db_desc_t.

In practice it appears all callers pass a structure large enough for the dereference to go beyond the end of 'uint32_t desc[2]' safely, but the compiler may optimize this away as undefined behaviour since it's an access outside the bounds of the array.

We need to test a fix like this:

diff --git a/nptl_db/fetch-value.c b/nptl_db/fetch-value.c
index 03b98cc5a0..2d95aa978f 100644
--- a/nptl_db/fetch-value.c
+++ b/nptl_db/fetch-value.c
@@ -140,7 +140,7 @@ _td_fetch_value (td_thragent_t *ta,
 
 td_err_e
 _td_store_value (td_thragent_t *ta,
-                uint32_t desc[2], int descriptor_name, psaddr_t idx,
+                db_desc_t desc, int descriptor_name, psaddr_t idx,
                 psaddr_t address, psaddr_t widened_value)
 {
   ps_err_e err;
@@ -240,7 +240,7 @@ _td_fetch_value_local (td_thragent_t *ta,
 
 td_err_e
 _td_store_value_local (td_thragent_t *ta,
-                      uint32_t desc[2], int descriptor_name, psaddr_t idx,
+                      db_desc_t desc, int descriptor_name, psaddr_t idx,
                       void *address, psaddr_t widened_value)
 {
   td_err_e terr = _td_locate_field (ta, desc, descriptor_name, idx, &address);
---
Comment 1 Florian Weimer 2019-01-04 07:37:22 UTC
There's no static keyword here, so I think C requires that the array size is ignored.  As C11 says:

A declaration of a parameter as “array of type” shall be adjusted to “qualified pointer to type”, where the type qualifiers (if any) are those specified within the [ and ] of the array type derivation.