[PATCH] libdwelf: provide a public debug section relocation function
Hey team elfutils,
This patch is for https://sourceware.org/bugzilla/show_bug.cgi?id=31447, and
it is ready for code review now 😀
Regards,
Di
From 5326db4241c15a4b2760d12bcc6b58aec86eda98 Mon Sep 17 00:00:00 2001
From: Di Chen
Date: Sun, 7 Jul 2024 14:57:30 +0800
Subject: [PATCH] libdwelf: provide a public debug section relocation function
When opening an ET_REL file through libdwfl, we do resolve all
cross-debug-section relocations using __libdwfl_relocate().
This commit provide a public function for the above feature, so users
can make sure that ET_REL files can be handled by libdw.
Here is an example:
int main(int argc, char **argv) {
const char *fname = argv[1];
Elf *elf;
elf_version(EV_CURRENT);
int fd = open(argv[1], O_RDWR);
elf = elf_begin(fd, ELF_C_RDWR, NULL);
dwelf_relocation_debug_sections(elf, fname);
elf_update(elf, ELF_C_WRITE);
return 0;
}
https://sourceware.org/bugzilla/show_bug.cgi?id=31447
Signed-off-by: Di Chen
---
libdw/libdw.map| 5 +
libdwelf/Makefile.am | 5 +-
libdwelf/dwelf_relocation_debug_sections.c | 389 +
libdwelf/libdwelf.h| 3 +
src/strip.c| 348 +-
tests/Makefile.am | 5 +-
tests/dwelf_reloc_dbg_scn.c| 25 ++
tests/run-dwelf-reloc-dbg-scn.sh | 48 +++
tests/testfile-dwelf-reloc-dbg-scn.o.bz2 | Bin 0 -> 1088 bytes
9 files changed, 481 insertions(+), 347 deletions(-)
create mode 100644 libdwelf/dwelf_relocation_debug_sections.c
create mode 100644 tests/dwelf_reloc_dbg_scn.c
create mode 100755 tests/run-dwelf-reloc-dbg-scn.sh
create mode 100644 tests/testfile-dwelf-reloc-dbg-scn.o.bz2
diff --git a/libdw/libdw.map b/libdw/libdw.map
index 552588a9..529036f8 100644
--- a/libdw/libdw.map
+++ b/libdw/libdw.map
@@ -383,3 +383,8 @@ ELFUTILS_0.192 {
global:
dwfl_set_sysroot;
} ELFUTILS_0.191;
+
+ELFUTILS_0.193 {
+ global:
+dwelf_relocation_debug_sections;
+} ELFUTILS_0.192;
diff --git a/libdwelf/Makefile.am b/libdwelf/Makefile.am
index a35a2873..a3d9cdd4 100644
--- a/libdwelf/Makefile.am
+++ b/libdwelf/Makefile.am
@@ -42,13 +42,14 @@ noinst_HEADERS = libdwelfP.h
libdwelf_a_SOURCES = dwelf_elf_gnu_debuglink.c dwelf_dwarf_gnu_debugaltlink.c \
dwelf_elf_gnu_build_id.c dwelf_scn_gnu_compressed_size.c \
dwelf_strtab.c dwelf_elf_begin.c \
- dwelf_elf_e_machine_string.c
+ dwelf_elf_e_machine_string.c \
+ dwelf_relocation_debug_sections.c
libdwelf = $(libdw)
libdw = ../libdw/libdw.so
libelf = ../libelf/libelf.so
-libebl = ../libebl/libebl.a
+libebl = ../libebl/libebl.a ../backends/libebl_backends.a
libeu = ../lib/libeu.a
libdwelf_pic_a_SOURCES =
diff --git a/libdwelf/dwelf_relocation_debug_sections.c b/libdwelf/dwelf_relocation_debug_sections.c
new file mode 100644
index ..b2949124
--- /dev/null
+++ b/libdwelf/dwelf_relocation_debug_sections.c
@@ -0,0 +1,389 @@
+#ifdef HAVE_CONFIG_H
+# include
+#endif
+
+#include
+#include "libdwP.h"
+#include "libebl.h"
+
+#define INTERNAL_ERROR(fname) \
+ error_exit(0, _("%s: INTERNAL ERROR %d (%s): %s"), fname, __LINE__, \
+ PACKAGE_VERSION, elf_errmsg(-1))
+
+typedef uint8_t GElf_Byte;
+static int debug_fd = -1;
+static char *tmp_debug_fname = NULL;
+
+
+const char *
+secndx_name (Elf *elf, size_t ndx)
+{
+ size_t shstrndx;
+ GElf_Shdr mem;
+ Elf_Scn *sec = elf_getscn (elf, ndx);
+ GElf_Shdr *shdr = gelf_getshdr (sec, &mem);
+ if (shdr == NULL || elf_getshdrstrndx (elf, &shstrndx) < 0)
+return "???";
+ return elf_strptr (elf, shstrndx, shdr->sh_name) ?: "???";
+}
+
+
+Elf_Data *
+get_xndxdata (Elf *elf, Elf_Scn *symscn)
+{
+ Elf_Data *xndxdata = NULL;
+ GElf_Shdr shdr_mem;
+ GElf_Shdr *shdr = gelf_getshdr (symscn, &shdr_mem);
+ if (shdr != NULL && shdr->sh_type == SHT_SYMTAB)
+{
+ size_t scnndx = elf_ndxscn (symscn);
+ Elf_Scn *xndxscn = NULL;
+ while ((xndxscn = elf_nextscn (elf, xndxscn)) != NULL)
+ {
+ GElf_Shdr xndxshdr_mem;
+ GElf_Shdr *xndxshdr = gelf_getshdr (xndxscn, &xndxshdr_mem);
+
+ if (xndxshdr != NULL
+ && xndxshdr->sh_type == SHT_SYMTAB_SHNDX
+ && xndxshdr->sh_link == scnndx)
+ {
+ xndxdata = elf_getdata (xndxscn, NULL);
+ break;
+ }
+ }
+}
+
+ return xndxdata;
+}
+
+
+void
+cleanup_debug (void)
+{
+ if (debug_fd >= 0)
+{
+ if (tmp_debug_fname != NULL)
+ {
+ unlink (tmp_debug_fname);
+ free (tmp_debug_fname);
+ tmp_debug_fname = NULL;
+ }
+ close (debug_fd);
+ debug_fd = -1;
+}
+}
+
+
+bool
+relocate (Elf *elf, GElf_Addr offset, const GElf_Sxword addend,
+ Elf_Data *tdata, unsigned int ei_data, const char *fname,
+ bool is_rela, GElf_Sym *sym, int addsub, Elf_Type type)
+{
+ /* These are the types we can relocate.
[Bug libdw/31447] Provide a public debug section relocation function
https://sourceware.org/bugzilla/show_bug.cgi?id=31447 --- Comment #7 from Di Chen --- This issue is in code review. https://sourceware.org/pipermail/elfutils-devel/2024q3/007388.html -- You are receiving this mail because: You are on the CC list for the bug.
[patch] debuginfod metadata query timeout enforcement
Hi -
Proposed patch for a problem we spotted recently in unreleased code:
commit b853004e723d058f97362dbfdc40d1c49ea2ef51 (HEAD -> main)
Author: Frank Ch. Eigler
Date: Tue Sep 3 11:27:36 2024 -0400
debuginfod: service metadata queries in separate, timed-out connections
The --metadata-maxtime=SECONDS parameter was intended to limit elapsed
time debuginfod spends attempting to answer metadata queries. These
can be slow because they have to apply glob matches across a large set
of strings, potentially taking many seconds.
However, this option was not implemented fully. It checked for
timeouts only when rows of data were finally served up by the
database, not during. Also, it used the same database connection that
normal debuginfod queries were using, locking them out.
New code creates a new temporary database connection for these
infrequent(?) metadata queries, and enforces timeouts using the
sqlite3 progress-handler callback. This effectively limits total
query runtime, inside or outside the database.
The elfutils testsuite dataset is not large enough to show either the
slow-glob or the locking-out-normal-queries phenomenon, so the
behavioral impact was hand-tested on a moderate sized debuginfod index
on a live federation server. A full valgrind leak check indicated
it's clean.
Signed-off-by: Frank Ch. Eigler
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index fb7873ae21d4..4bb517bde80f 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1126,7 +1126,10 @@ struct sqlite_ps
const string nickname;
const string sql;
sqlite3_stmt *pp;
-
+ // for step_timeout()/callback
+ struct timespec ts_start;
+ double ts_timeout;
+
sqlite_ps(const sqlite_ps&); // make uncopyable
sqlite_ps& operator=(const sqlite_ps &); // make unassignable
@@ -1138,6 +1141,7 @@ struct sqlite_ps
int rc = sqlite3_prepare_v2 (db, sql.c_str(), -1 /* to \0 */, & this->pp,
NULL);
if (rc != SQLITE_OK)
throw sqlite_exception(rc, "prepare " + sql);
+this->reset_timeout(0.0);
}
sqlite_ps& reset()
@@ -1197,6 +1201,46 @@ struct sqlite_ps
return rc;
}
+
+ void reset_timeout(double s) // set starting point for maximum elapsed time
in step_timeouts()
+ {
+clock_gettime (CLOCK_MONOTONIC, &this->ts_start);
+this->ts_timeout = s;
+ }
+
+
+ static int sqlite3_progress_handler_cb (void *param)
+ {
+sqlite_ps *pp = (sqlite_ps*) param;
+struct timespec ts_end;
+clock_gettime (CLOCK_MONOTONIC, &ts_end);
+double deltas = (ts_end.tv_sec - pp->ts_start.tv_sec) + (ts_end.tv_nsec -
pp->ts_start.tv_nsec)/1.e9;
+return (interrupted || (deltas > pp->ts_timeout)); // non-zero =>
interrupt sqlite operation in progress
+ }
+
+
+ int step_timeout() {
+// Do the same thing as step(), except wrapping it into a timeout
+// relative to the last reset_timeout() invocation.
+//
+// Do this by attaching a progress_handler to the database
+// connection, for the duration of this operation. It should be a
+// private connection to the calling thread, so other operations
+// cannot begin concurrently.
+
+sqlite3_progress_handler(this->db, 1 /* bytecode insns */,
+ & sqlite3_progress_handler_cb, (void*) this);
+int rc = this->step();
+sqlite3_progress_handler(this->db, 0, 0, 0); // disable
+struct timespec ts_end;
+clock_gettime (CLOCK_MONOTONIC, &ts_end);
+double deltas = (ts_end.tv_sec - this->ts_start.tv_sec) + (ts_end.tv_nsec
- this->ts_start.tv_nsec)/1.e9;
+if (verbose > 3)
+ obatched(clog) << this->nickname << " progress-delta-final " << deltas
<< endl;
+return rc;
+ }
+
+
~sqlite_ps () { sqlite3_finalize (this->pp); }
operator sqlite3_stmt* () { return this->pp; }
};
@@ -3503,8 +3547,19 @@ handle_metadata (MHD_Connection* conn,
string key, string value, off_t* size)
{
MHD_Response* r;
- sqlite3 *thisdb = dbq;
-
+ // Because this query can take on the order of many seconds, we need
+ // to prevent DoS against the other normal quick queries, so we use
+ // a dedicated database connection.
+ sqlite3 *thisdb = 0;
+ int rc = sqlite3_open_v2 (db_path.c_str(), &thisdb, (SQLITE_OPEN_READONLY
+ |SQLITE_OPEN_URI
+
|SQLITE_OPEN_PRIVATECACHE
+ |SQLITE_OPEN_NOMUTEX),
/* private to us */
+NULL);
+ if (rc)
+throw sqlite_exception(rc, "cannot open database for metadata query");
+ defer_dtor sqlite_db_closer (thisdb, sqlite3_close_v2);
+
// Query locally for matching e, d files
string op;
if (key == "glob")
@@ -3579,7 +3634,8 @@ handle_metadata (MHD_Con
