From patchwork Tue Jan 14 21:09:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 37384 Received: (qmail 129224 invoked by alias); 14 Jan 2020 21:10:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 129200 invoked by uid 89); 14 Jan 2020 21:10:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.4 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=HContent-Transfer-Encoding:8bit X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 14 Jan 2020 21:10:01 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id E6BBC117507; Tue, 14 Jan 2020 16:09:59 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id Nd9YOYlWak0S; Tue, 14 Jan 2020 16:09:59 -0500 (EST) Received: from murgatroyd.Home (75-166-123-50.hlrn.qwest.net [75.166.123.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPSA id A15331173BF; Tue, 14 Jan 2020 16:09:59 -0500 (EST) From: Tom Tromey To: gdb-patches@sourceware.org Cc: Tom Tromey Subject: [PATCH 1/3] Avoid hash table corruption in gdb_bfd.c Date: Tue, 14 Jan 2020 14:09:54 -0700 Message-Id: <20200114210956.25115-2-tromey@adacore.com> In-Reply-To: <20200114210956.25115-1-tromey@adacore.com> References: <20200114210956.25115-1-tromey@adacore.com> MIME-Version: 1.0 gdb caches BFDs that come from ordinary files. This code turns out to have a bug where the hash table can become corrupted, causing gdb to crash. When gdb_bfd_open opens the BFD, it uses fstat to get the BFD's mtime. This is used when inserting the entry into gdb_bfd_cache. Then, the function creates the gdb_bfd_data object as a side effect of calling new_reference. This object is used when finding objects in the hash table, and its constructor uses bfd_get_mtime. So, if the file changes between the time the BFD is put into the cache and the time that this object is created, the hash table will be incorrect. When the BFD is later deleted, its entry in the hash table will not be found, and at this point the hash table will point to invalid memory. This patch fixes the bug by ensuring that the mtime that is used for insertion is also used when creating the gdb_bfd_data. gdb/ChangeLog 2020-01-14 Tom Tromey PR win32/25302: * gdb_bfd.c (gdb_bfd_data): Add "mt" parameter. (gdb_bfd_init_data): New function. (gdb_bfd_open, gdb_bfd_ref): Use gdb_bfd_init_data. Change-Id: I649ef7060651ac12188fa99d6ae1546caec93594 --- gdb/ChangeLog | 7 +++++++ gdb/gdb_bfd.c | 50 +++++++++++++++++++++++++++++++++++--------------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c index 5a6dee2d51a..a1ee7814f32 100644 --- a/gdb/gdb_bfd.c +++ b/gdb/gdb_bfd.c @@ -59,8 +59,8 @@ static htab_t all_bfds; struct gdb_bfd_data { - gdb_bfd_data (bfd *abfd) - : mtime (bfd_get_mtime (abfd)), + gdb_bfd_data (bfd *abfd, time_t mt) + : mtime (mt), size (bfd_get_size (abfd)), relocation_computed (0), needs_relocations (0), @@ -376,6 +376,30 @@ gdb_bfd_iovec_fileio_fstat (struct bfd *abfd, void *stream, return result; } +/* A helper function to initialize the data that gdb attaches to each + BFD. */ + +static void +gdb_bfd_init_data (struct bfd *abfd, time_t mtime) +{ + struct gdb_bfd_data *gdata; + void **slot; + + gdb_assert (bfd_usrdata (abfd) == nullptr); + + /* Ask BFD to decompress sections in bfd_get_full_section_contents. */ + abfd->flags |= BFD_DECOMPRESS; + + gdata = new gdb_bfd_data (abfd, mtime); + bfd_set_usrdata (abfd, gdata); + bfd_alloc_data (abfd); + + /* This is the first we've seen it, so add it to the hash table. */ + slot = htab_find_slot (all_bfds, abfd, INSERT); + gdb_assert (slot && !*slot); + *slot = abfd; +} + /* See gdb_bfd.h. */ gdb_bfd_ref_ptr @@ -469,7 +493,14 @@ gdb_bfd_open (const char *name, const char *target, int fd) *slot = abfd; } - return gdb_bfd_ref_ptr::new_reference (abfd); + /* It's important to pass the already-computed mtime here, rather + than, say, calling gdb_bfd_ref_ptr::new_reference here. BFD by + default will "stat" the file each time bfd_get_mtime is called -- + and since we already entered it into the hash table using this + mtime, if the file changed at the wrong moment, the race would + lead to a hash table corruption. */ + gdb_bfd_init_data (abfd, search.mtime); + return gdb_bfd_ref_ptr (abfd); } /* A helper function that releases any section data attached to the @@ -522,7 +553,6 @@ void gdb_bfd_ref (struct bfd *abfd) { struct gdb_bfd_data *gdata; - void **slot; if (abfd == NULL) return; @@ -541,17 +571,7 @@ gdb_bfd_ref (struct bfd *abfd) return; } - /* Ask BFD to decompress sections in bfd_get_full_section_contents. */ - abfd->flags |= BFD_DECOMPRESS; - - gdata = new gdb_bfd_data (abfd); - bfd_set_usrdata (abfd, gdata); - bfd_alloc_data (abfd); - - /* This is the first we've seen it, so add it to the hash table. */ - slot = htab_find_slot (all_bfds, abfd, INSERT); - gdb_assert (slot && !*slot); - *slot = abfd; + gdb_bfd_init_data (abfd, bfd_get_mtime (abfd)); } /* See gdb_bfd.h. */