[Crash-utility] [PATCH v1 2/3] symbols: Fix potential read to already freed object

HATAYAMA Daisuke d.hatayama at fujitsu.com
Mon Jan 4 05:28:44 UTC 2021


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 at 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




More information about the Crash-utility mailing list