[Crash-utility] [PATCH v2 2/3] symbols: Fix potential read to already freed object
valgrind detects the following potential invalid read to some already
freed object:
==22220== Invalid read of size 4
==22220== at 0x539641: datatype_info (symbols.c:5791)
==22220== by 0x4EC8B1: dump_variable_length_record_log (kernel.c:5313)
==22220== by 0x4EC8B1: dump_log (kernel.c:5042)
==22220== by 0x4C5A25: get_panicmsg (task.c:6275)
==22220== by 0x4F3E71: display_sys_stats (kernel.c:5645)
==22220== by 0x464BC7: main_loop (main.c:797)
==22220== by 0x6BF262: captured_command_loop (main.c:258)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C02E5: captured_main (main.c:1064)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C0596: gdb_main (main.c:1079)
==22220== by 0x6C0596: gdb_main_entry (main.c:1099)
==22220== by 0x46316F: main (main.c:708)
==22220== Address 0xb498c8 is 72 bytes inside a block of size 1,024 free'd
==22220== at 0x471261: freebuf (tools.c:5766)
==22220== by 0x53946B: datatype_info (symbols.c:5747)
==22220== by 0x4FEA2A: net_init (net.c:173)
==22220== by 0x464A55: main_loop (main.c:777)
==22220== by 0x6BF262: captured_command_loop (main.c:258)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C02E5: captured_main (main.c:1064)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C0596: gdb_main (main.c:1079)
==22220== by 0x6C0596: gdb_main_entry (main.c:1099)
==22220== by 0x46316F: main (main.c:708)
==22220== Block was alloc'd at
==22220== at 0x471C80: getbuf (tools.c:5965)
==22220== by 0x5392B7: datatype_info (symbols.c:5624)
==22220== by 0x4FEA2A: net_init (net.c:173)
==22220== by 0x464A55: main_loop (main.c:777)
==22220== by 0x6BF262: captured_command_loop (main.c:258)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C02E5: captured_main (main.c:1064)
==22220== by 0x6BD869: catch_errors (exceptions.c:557)
==22220== by 0x6C0596: gdb_main (main.c:1079)
==22220== by 0x6C0596: gdb_main_entry (main.c:1099)
==22220== by 0x46316F: main (main.c:708)
==22220==
This was caused by the fact that in datatype_info(), the object
associated with the variable req is freed too early although it's
still be referred to after the freeing.
Fix this by changing the way allocating the object from by GETBUF() to
by allocation on stack, which simplifies the code because explicit
free() operations are unnecessary.
Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com>
---
symbols.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/symbols.c b/symbols.c
index a51078d..c0f68af 100644
--- a/symbols.c
+++ b/symbols.c
@@ -5611,7 +5611,7 @@ datatype_init(void)
long
datatype_info(char *name, char *member, struct datatype_member *dm)
{
- struct gnu_request *req;
+ struct gnu_request request, *req = &request;
long offset, size, member_size;
int member_typecode;
ulong type_found;
@@ -5625,7 +5625,7 @@ datatype_info(char *name, char *member, struct datatype_member *dm)
strcpy(buf, name);
- req = (struct gnu_request *)GETBUF(sizeof(struct gnu_request));
+ BZERO(req, sizeof(*req));
req->command = GNU_GET_DATATYPE;
req->flags |= GNU_RETURN_ON_ERROR;
req->name = buf;
@@ -5633,10 +5633,8 @@ datatype_info(char *name, char *member, struct datatype_member
*dm)
req->fp = pc->nullfp;
gdb_interface(req);
- if (req->flags & GNU_COMMAND_FAILED) {
- FREEBUF(req);
+ if (req->flags & GNU_COMMAND_FAILED)
return (dm == MEMBER_TYPE_NAME_REQUEST) ? 0 : -1;
- }
if (!req->typecode) {
sprintf(buf, "struct %s", name);
@@ -5748,8 +5746,6 @@ datatype_info(char *name, char *member, struct datatype_member *dm)
break;
}
- FREEBUF(req);
-
if (dm && (dm != MEMBER_SIZE_REQUEST) && (dm !=
MEMBER_TYPE_REQUEST) &&
(dm != STRUCT_SIZE_REQUEST) && (dm != MEMBER_TYPE_NAME_REQUEST)) {
dm->type = type_found;
--
1.8.3.1