* [PATCH v3] debuginfod: Support queries for ELF/DWARF sections
@ 2022-10-28 3:59 Aaron Merey
2022-10-30 0:29 ` Mark Wielaard
0 siblings, 1 reply; 4+ messages in thread
From: Aaron Merey @ 2022-10-28 3:59 UTC (permalink / raw)
To: elfutils-devel; +Cc: Aaron Merey
v3 addresses Frank and Mark's v2 feedback.
v2 available here:
https://sourceware.org/pipermail/elfutils-devel/2022q4/005476.html
---
ChangeLog | 4 +
NEWS | 2 +
debuginfod/ChangeLog | 21 ++
debuginfod/Makefile.am | 2 +-
debuginfod/debuginfod-client.c | 263 +++++++++++++++++++++-
debuginfod/debuginfod-find.c | 21 +-
debuginfod/debuginfod.cxx | 383 ++++++++++++++++++++++++++------
debuginfod/debuginfod.h.in | 6 +
debuginfod/libdebuginfod.map | 1 +
doc/ChangeLog | 7 +
doc/Makefile.am | 1 +
doc/debuginfod.8 | 10 +
doc/debuginfod_find_debuginfo.3 | 22 +-
doc/debuginfod_find_section.3 | 1 +
tests/ChangeLog | 5 +
tests/Makefile.am | 4 +-
tests/run-debuginfod-section.sh | 134 +++++++++++
17 files changed, 804 insertions(+), 83 deletions(-)
create mode 100644 doc/debuginfod_find_section.3
create mode 100755 tests/run-debuginfod-section.sh
diff --git a/ChangeLog b/ChangeLog
index 60624183..6f9b416a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2022-10-27 Aaron Merey <amerey@redhat.com>
+
+ * NEWS: Add debuginfod_find_section.
+
2022-09-13 Aleksei Vetrov <vvvvvv@google.com>
* NEWS (libdwfl): Add dwfl_report_offline_memory.
diff --git a/NEWS b/NEWS
index 6ebd172c..3e290ff7 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,8 @@ readelf: Add -D, --use-dynamic option.
debuginfod: Add --disable-source-scan option.
+debuginfod-client: Add new function debuginfod_find_section.
+
libdwfl: Add new function dwfl_get_debuginfod_client.
Add new function dwfl_frame_reg.
Add new function dwfl_report_offline_memory.
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 59d50df1..b1d076fe 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,24 @@
+2022-10-27 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (libdebuginfod_so_LDLIBS): Add libelf.
+ * debuginfod-client.c (debuginfod_find_section): New function.
+ (extract_section): New function.
+ (cache_find_section): New function.
+ (debuginfod_query_server): Add support for section queries.
+ * debuginfod-find.c (main): Add support for section queries.
+ * debuginfod.cxx (extract_section): New function.
+ (handle_buildid_f_match): Add section parameter. When non-empty,
+ try to create response from section contents.
+ (handle_buildid_r_match): Add section parameter. When non-empty,
+ try to create response from section contents.
+ (handle_buildid_match): Add section parameter. Pass to
+ handle_buildid_{f,r}_match.
+ (handle_buildid): Handle section name when artifacttype is set to
+ "section". Query upstream servers via debuginfod_find_section
+ when necessary.
+ (debuginfod.h.in): Add declaration for debuginfod_find_section.
+ (libdebuginfod.map): Add debuginfod_find_section.
+
2022-10-17 Frank Ch. Eigler <fche@redhat.com>
* debuginfod.cxx (main): Report libmicrohttpd version.
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 435cb8a6..f27d6e2e 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -97,7 +97,7 @@ libdebuginfod_so_LIBS = libdebuginfod_pic.a
if DUMMY_LIBDEBUGINFOD
libdebuginfod_so_LDLIBS =
else
-libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS)
+libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS) $(libelf)
endif
$(LIBDEBUGINFOD_SONAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 2a14d9d9..0e8ca89c 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -44,6 +44,7 @@
#include "system.h"
#include <errno.h>
#include <stdlib.h>
+#include <libdwelf.h>
/* We might be building a bootstrap dummy library, which is really simple. */
#ifdef DUMMY_LIBDEBUGINFOD
@@ -55,6 +56,9 @@ int debuginfod_find_executable (debuginfod_client *c, const unsigned char *b,
int s, char **p) { return -ENOSYS; }
int debuginfod_find_source (debuginfod_client *c, const unsigned char *b,
int s, const char *f, char **p) { return -ENOSYS; }
+int debuginfod_find_section (debuginfod_client *c, const unsigned char *b,
+ int s, const char *scn, char **p)
+ { return -ENOSYS; }
void debuginfod_set_progressfn(debuginfod_client *c,
debuginfod_progressfn_t fn) { }
void debuginfod_set_verbose_fd(debuginfod_client *c, int fd) { }
@@ -129,6 +133,9 @@ struct debuginfod_client
debuginfod_end needs to terminate. */
int default_progressfn_printed_p;
+ /* Indicates whether the last query was cancelled by progressfn. */
+ bool progressfn_cancel;
+
/* File descriptor to output any verbose messages if > 0. */
int verbose_fd;
@@ -536,6 +543,167 @@ header_callback (char * buffer, size_t size, size_t numitems, void * userdata)
return numitems;
}
+/* Attempt to read an ELF/DWARF section with name SECTION from FD and write
+ it to a separate file in the debuginfod cache. If successful the absolute
+ path of the separate file containing SECTION will be stored in USR_PATH.
+ FD_PATH is the absolute path for FD.
+
+ If the section cannot be extracted, then return a negative error code.
+ -ENOENT indicates that the parent file was able to be read but the
+ section name was not found. -EEXIST indicates that the section was
+ found but had type SHT_NOBITS. */
+
+int
+extract_section (int fd, const char *section, char *fd_path, char **usr_path)
+{
+ Elf *elf = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, NULL);
+ if (elf == NULL)
+ return -EIO;
+
+ size_t shstrndx;
+ int rc = elf_getshdrstrndx (elf, &shstrndx);
+ if (rc < 0)
+ return -EIO;
+
+ /* Try to find the target section and copy the contents into a
+ separate file. */
+ Elf_Scn *scn = NULL;
+ while (true)
+ {
+ scn = elf_nextscn (elf, scn);
+ if (scn == NULL)
+ {
+ rc = -ENOENT;
+ break;
+ }
+ GElf_Shdr shdr_storage;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_storage);
+ if (shdr == NULL)
+ {
+ rc = -EIO;
+ break;
+ }
+
+ const char *scn_name = elf_strptr (elf, shstrndx, shdr->sh_name);
+ if (scn_name == NULL)
+ {
+ rc = -EIO;
+ break;
+ }
+ if (strcmp (scn_name, section) == 0)
+ {
+ /* We found the desired section. */
+ if (shdr->sh_type == SHT_NOBITS)
+ {
+ rc = -EEXIST;
+ break;
+ }
+
+ Elf_Data *data = NULL;
+ data = elf_getdata (scn, NULL);
+ if (data == NULL)
+ {
+ rc = -EIO;
+ break;
+ }
+
+ if (data->d_buf == NULL)
+ {
+ rc = -EIO;
+ break;
+ }
+
+ /* Compute the absolute filename we'll write the section to. Replace
+ the last component of FD_PATH with the section filename. */
+ int i = strlen (fd_path);
+ while (i >= 0)
+ {
+ if (fd_path[i] == '/')
+ {
+ fd_path[i] = '\0';
+ break;
+ }
+ --i;
+ }
+
+ char *sec_path;
+ if (asprintf (&sec_path, "%s/section-%s", fd_path, section) == -1)
+ {
+ rc = -ENOMEM;
+ break;
+ }
+
+ int sec_fd = open (sec_path, O_CREAT | O_EXCL | O_RDWR, S_IRUSR);
+ if (sec_fd < 0)
+ {
+ free (sec_path);
+ rc = -errno;
+ break;
+ }
+
+ ssize_t res = write_retry (sec_fd, data->d_buf, data->d_size);
+ if (res < 0 || (size_t) res != data->d_size)
+ {
+ close (sec_fd);
+ free (sec_path);
+ rc = -EIO;
+ break;
+ }
+
+ /* Success. Update USR_PATH. */
+ if (usr_path != NULL)
+ *usr_path = sec_path;
+ else
+ free (sec_path);
+ rc = sec_fd;
+ break;
+ }
+ }
+
+ elf_end (elf);
+ return rc;
+}
+
+/* Search TARGET_CACHE_DIR for a debuginfo or executable file containing
+ an ELF/DWARF section with name SCN_NAME. If found, extract the section
+ to a separate file in TARGET_CACHE_DIR and return a file descriptor
+ for the section file. The path for this file will be stored in USR_PATH.
+ Return a negative errno if unsuccessful. */
+
+static int
+cache_find_section (const char *scn_name, const char *target_cache_dir,
+ char **usr_path)
+{
+ int fd;
+ int rc = -EEXIST;
+ char debug_path[PATH_MAX];
+
+ /* Check the debuginfo first. */
+ snprintf (debug_path, PATH_MAX, "%s/debuginfo", target_cache_dir);
+ fd = open (debug_path, O_RDONLY);
+ if (fd >= 0)
+ {
+ rc = extract_section (fd, scn_name, debug_path, usr_path);
+ close (fd);
+ }
+
+ /* If the section type was SHT_NOBITS, check the executable. */
+ if (rc == -EEXIST)
+ {
+ char exec_path[PATH_MAX];
+ snprintf (exec_path, PATH_MAX, "%s/executable", target_cache_dir);
+ fd = open (exec_path, O_RDONLY);
+
+ if (fd >= 0)
+ {
+ rc = extract_section (fd, scn_name, exec_path, usr_path);
+ close (fd);
+ }
+ }
+
+ return rc;
+}
+
/* Query each of the server URLs found in $DEBUGINFOD_URLS for the file
with the specified build-id, type (debuginfo, executable or source)
and filename. filename may be NULL. If found, return a file
@@ -546,11 +714,13 @@ debuginfod_query_server (debuginfod_client *c,
const unsigned char *build_id,
int build_id_len,
const char *type,
- const char *filename,
+ const char *type_arg,
char **path)
{
char *server_urls;
char *urls_envvar;
+ const char *section = NULL;
+ const char *filename = NULL;
char *cache_path = NULL;
char *maxage_path = NULL;
char *interval_path = NULL;
@@ -563,6 +733,17 @@ debuginfod_query_server (debuginfod_client *c,
int vfd = c->verbose_fd;
int rc;
+ c->progressfn_cancel = false;
+
+ if (strcmp (type, "source") == 0)
+ filename = type_arg;
+ else if (strcmp (type, "section") == 0)
+ {
+ section = type_arg;
+ if (section == NULL)
+ return -EINVAL;
+ }
+
if (vfd >= 0)
{
dprintf (vfd, "debuginfod_find_%s ", type);
@@ -681,6 +862,8 @@ debuginfod_query_server (debuginfod_client *c,
PATH_MAX and truncate/collide. Oh well, that'll teach
them! */
}
+ else if (section != NULL)
+ strncpy (suffix, section, PATH_MAX);
else
suffix[0] = '\0';
@@ -752,7 +935,10 @@ debuginfod_query_server (debuginfod_client *c,
}
xalloc_str (target_cache_dir, "%s/%s", cache_path, build_id_bytes);
- xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
+ if (section != NULL)
+ xalloc_str (target_cache_path, "%s/%s-%s", target_cache_dir, type, suffix);
+ else
+ xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
xalloc_str (target_cache_tmppath, "%s.XXXXXX", target_cache_path);
/* XXX combine these */
@@ -836,6 +1022,18 @@ debuginfod_query_server (debuginfod_client *c,
/* Ensure old 000-permission files are not lingering in the cache. */
unlink(target_cache_path);
+ if (section != NULL)
+ {
+ /* Try to extract the section from a cached file before querying
+ any servers. */
+ rc = cache_find_section (section, target_cache_dir, path);
+
+ /* If the section was found or confirmed to not exist, then we
+ are done. */
+ if (rc >= 0 || rc == -ENOENT)
+ goto out;
+ }
+
long timeout = default_timeout;
const char* timeout_envvar = getenv(DEBUGINFOD_TIMEOUT_ENV_VAR);
if (timeout_envvar != NULL)
@@ -1008,6 +1206,9 @@ debuginfod_query_server (debuginfod_client *c,
snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
build_id_bytes, type, escaped_string);
}
+ else if (section)
+ snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
+ build_id_bytes, type, section);
else
snprintf(data[i].url, PATH_MAX, "%s/%s/%s", server_url, build_id_bytes, type);
if (vfd >= 0)
@@ -1212,7 +1413,10 @@ debuginfod_query_server (debuginfod_client *c,
}
if ((*c->progressfn) (c, pa, dl_size))
- break;
+ {
+ c->progressfn_cancel = true;
+ break;
+ }
}
/* Check to see if we are downloading something which exceeds maxsize, if set.*/
@@ -1279,6 +1483,8 @@ debuginfod_query_server (debuginfod_client *c,
/* 406 signals that the requested file was too large */
if ( ok0 == CURLE_OK && resp_code == 406)
rc = -EFBIG;
+ else if (section != NULL && resp_code == 503)
+ rc = -EINVAL;
else
rc = -ENOENT;
break;
@@ -1525,6 +1731,7 @@ debuginfod_begin (void)
goto out1;
}
+ elf_version (EV_CURRENT);
// extra future initialization
goto out;
@@ -1604,6 +1811,56 @@ int debuginfod_find_source(debuginfod_client *client,
"source", filename, path);
}
+int
+debuginfod_find_section (debuginfod_client *client,
+ const unsigned char *build_id, int build_id_len,
+ const char *section, char **path)
+{
+ int rc = debuginfod_query_server(client, build_id, build_id_len,
+ "section", section, path);
+ if (rc != -EINVAL)
+ return rc;
+
+ /* The servers may have lacked support for section queries. Attempt to
+ download the debuginfo or executable containing the section in order
+ to extract it. */
+ rc = -EEXIST;
+ int fd = -1;
+ char *tmp_path = NULL;
+
+ fd = debuginfod_find_debuginfo (client, build_id, build_id_len, &tmp_path);
+ if (client->progressfn_cancel)
+ {
+ if (fd >= 0)
+ {
+ /* This shouldn't happen, but we'll check this condition
+ just in case. */
+ close (fd);
+ free (tmp_path);
+ }
+ return -ENOENT;
+ }
+ if (fd > 0)
+ {
+ rc = extract_section (fd, section, tmp_path, path);
+ close (fd);
+ }
+
+ if (rc == -EEXIST)
+ {
+ /* The section should be found in the executable. */
+ fd = debuginfod_find_executable (client, build_id,
+ build_id_len, &tmp_path);
+ if (fd > 0)
+ {
+ rc = extract_section (fd, section, tmp_path, path);
+ close (fd);
+ }
+ }
+
+ free (tmp_path);
+ return rc;
+}
/* Add an outgoing HTTP header. */
int debuginfod_add_http_header (debuginfod_client *client, const char* header)
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 778fb09b..cf6e69d0 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -190,13 +190,11 @@ main(int argc, char** argv)
debuginfod_find_* function. If FILETYPE is "source"
then ensure a FILENAME was also supplied as an argument. */
if (strcmp(argv[remaining], "debuginfo") == 0)
- rc = debuginfod_find_debuginfo(client,
- build_id, build_id_len,
- &cache_name);
+ rc = debuginfod_find_debuginfo(client, build_id,
+ build_id_len, &cache_name);
else if (strcmp(argv[remaining], "executable") == 0)
- rc = debuginfod_find_executable(client,
- build_id, build_id_len,
- &cache_name);
+ rc = debuginfod_find_executable(client, build_id,
+ build_id_len, &cache_name);
else if (strcmp(argv[remaining], "source") == 0)
{
if (remaining+2 == argc || argv[remaining+2][0] != '/')
@@ -208,6 +206,17 @@ main(int argc, char** argv)
build_id, build_id_len,
argv[remaining+2], &cache_name);
}
+ else if (strcmp(argv[remaining], "section") == 0)
+ {
+ if (remaining+2 == argc)
+ {
+ fprintf(stderr,
+ "If FILETYPE is \"section\" then a section name must be given\n");
+ return 1;
+ }
+ rc = debuginfod_find_section(client, build_id, build_id_len,
+ argv[remaining+2], &cache_name);
+ }
else
{
argp_help (&argp, stderr, ARGP_HELP_USAGE, argv[0]);
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 9dc4836b..5e1dec05 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1136,66 +1136,6 @@ add_mhd_last_modified (struct MHD_Response *resp, time_t mtime)
add_mhd_response_header (resp, "Cache-Control", "public");
}
-
-
-static struct MHD_Response*
-handle_buildid_f_match (bool internal_req_t,
- int64_t b_mtime,
- const string& b_source0,
- int *result_fd)
-{
- (void) internal_req_t; // ignored
- int fd = open(b_source0.c_str(), O_RDONLY);
- if (fd < 0)
- throw libc_exception (errno, string("open ") + b_source0);
-
- // NB: use manual close(2) in error case instead of defer_dtor, because
- // in the normal case, we want to hand the fd over to libmicrohttpd for
- // file transfer.
-
- struct stat s;
- int rc = fstat(fd, &s);
- if (rc < 0)
- {
- close(fd);
- throw libc_exception (errno, string("fstat ") + b_source0);
- }
-
- if ((int64_t) s.st_mtime != b_mtime)
- {
- if (verbose)
- obatched(clog) << "mtime mismatch for " << b_source0 << endl;
- close(fd);
- return 0;
- }
-
- inc_metric ("http_responses_total","result","file");
- struct MHD_Response* r = MHD_create_response_from_fd ((uint64_t) s.st_size, fd);
- if (r == 0)
- {
- if (verbose)
- obatched(clog) << "cannot create fd-response for " << b_source0 << endl;
- close(fd);
- }
- else
- {
- std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length());
- add_mhd_response_header (r, "Content-Type", "application/octet-stream");
- add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
- to_string(s.st_size).c_str());
- add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
- add_mhd_last_modified (r, s.st_mtime);
- if (verbose > 1)
- obatched(clog) << "serving file " << b_source0 << endl;
- /* libmicrohttpd will close it. */
- if (result_fd)
- *result_fd = fd;
- }
-
- return r;
-}
-
-
// quote all questionable characters of str for safe passage through a sh -c expansion.
static string
shell_escape(const string& str)
@@ -1589,6 +1529,205 @@ public:
};
static libarchive_fdcache fdcache;
+/* Search ELF_FD for an ELF/DWARF section with name SECTION.
+ If found copy the section to a temporary file and return
+ its file descriptor, otherwise return -1.
+
+ The temporary file's mtime will be set to PARENT_MTIME.
+ B_SOURCE should be a description of the parent file suitable
+ for printing to the log. */
+
+static int
+extract_section (int elf_fd, int64_t parent_mtime,
+ const string& b_source, const string& section)
+{
+ /* Search the fdcache. */
+ struct stat fs;
+ int fd = fdcache.lookup (b_source, section);
+ if (fd >= 0)
+ {
+ if (fstat (fd, &fs) != 0)
+ {
+ if (verbose)
+ obatched (clog) << "cannot fstate fdcache "
+ << b_source << " " << section << endl;
+ close (fd);
+ return -1;
+ }
+ if ((int64_t) fs.st_mtime != parent_mtime)
+ {
+ if (verbose)
+ obatched(clog) << "mtime mismatch for "
+ << b_source << " " << section << endl;
+ close (fd);
+ return -1;
+ }
+ /* Success. */
+ return fd;
+ }
+
+ Elf *elf = elf_begin (elf_fd, ELF_C_READ_MMAP_PRIVATE, NULL);
+ if (elf == NULL)
+ return -1;
+
+ /* Try to find the section and copy the contents into a separate file. */
+ try
+ {
+ size_t shstrndx;
+ int rc = elf_getshdrstrndx (elf, &shstrndx);
+ if (rc < 0)
+ throw elfutils_exception (rc, "getshdrstrndx");
+
+ Elf_Scn *scn = NULL;
+ while (true)
+ {
+ scn = elf_nextscn (elf, scn);
+ if (scn == NULL)
+ break;
+ GElf_Shdr shdr_storage;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_storage);
+ if (shdr == NULL)
+ break;
+
+ const char *scn_name = elf_strptr (elf, shstrndx, shdr->sh_name);
+ if (scn_name == NULL)
+ break;
+ if (scn_name == section)
+ {
+ Elf_Data *data = NULL;
+
+ /* We found the desired section. */
+ data = elf_getdata (scn, NULL);
+ if (data == NULL)
+ throw elfutils_exception (elf_errno (), "getdata");
+ if (data->d_buf == NULL)
+ {
+ obatched(clog) << "section " << section
+ << " is empty" << endl;
+ break;
+ }
+
+ /* Create temporary file containing the section. */
+ char *tmppath = NULL;
+ rc = asprintf (&tmppath, "%s/debuginfod.XXXXXX", tmpdir.c_str());
+ if (rc < 0)
+ throw libc_exception (ENOMEM, "cannot allocate tmppath");
+ defer_dtor<void*,void> tmmpath_freer (tmppath, free);
+ fd = mkstemp (tmppath);
+ if (fd < 0)
+ throw libc_exception (errno, "cannot create temporary file");
+ ssize_t res = write_retry (fd, data->d_buf, data->d_size);
+ if (res < 0 || (size_t) res != data->d_size)
+ throw libc_exception (errno, "cannot write to temporary file");
+
+ /* Set mtime to be the same as the parent file's mtime. */
+ struct timeval tvs[2];
+ if (fstat (elf_fd, &fs) != 0)
+ throw libc_exception (errno, "cannot fstat file");
+
+ tvs[0].tv_sec = tvs[1].tv_sec = fs.st_mtime;
+ tvs[0].tv_usec = tvs[1].tv_usec = 0;
+ (void) futimes (fd, tvs);
+
+ /* Add to fdcache. */
+ fdcache.intern (b_source, section, tmppath, data->d_size, true);
+ break;
+ }
+ }
+ }
+ catch (const reportable_exception &e)
+ {
+ e.report (clog);
+ close (fd);
+ fd = -1;
+ }
+
+ elf_end (elf);
+ return fd;
+}
+
+static struct MHD_Response*
+handle_buildid_f_match (bool internal_req_t,
+ int64_t b_mtime,
+ const string& b_source0,
+ const string& section,
+ int *result_fd)
+{
+ (void) internal_req_t; // ignored
+ int fd = open(b_source0.c_str(), O_RDONLY);
+ if (fd < 0)
+ throw libc_exception (errno, string("open ") + b_source0);
+
+ // NB: use manual close(2) in error case instead of defer_dtor, because
+ // in the normal case, we want to hand the fd over to libmicrohttpd for
+ // file transfer.
+
+ struct stat s;
+ int rc = fstat(fd, &s);
+ if (rc < 0)
+ {
+ close(fd);
+ throw libc_exception (errno, string("fstat ") + b_source0);
+ }
+
+ if ((int64_t) s.st_mtime != b_mtime)
+ {
+ if (verbose)
+ obatched(clog) << "mtime mismatch for " << b_source0 << endl;
+ close(fd);
+ return 0;
+ }
+
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, s.st_mtime, b_source0, section);
+ close (fd);
+
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for " << b_source0 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &s);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno, string ("fstat ") + b_source0
+ + string (" ") + section);
+ }
+ }
+
+ struct MHD_Response* r = MHD_create_response_from_fd ((uint64_t) s.st_size, fd);
+ inc_metric ("http_responses_total","result","file");
+ if (r == 0)
+ {
+ if (verbose)
+ obatched(clog) << "cannot create fd-response for " << b_source0
+ << " section=" << section << endl;
+ close(fd);
+ }
+ else
+ {
+ std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length());
+ add_mhd_response_header (r, "Content-Type", "application/octet-stream");
+ add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
+ to_string(s.st_size).c_str());
+ add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
+ add_mhd_last_modified (r, s.st_mtime);
+ if (verbose > 1)
+ obatched(clog) << "serving file " << b_source0 << " section=" << section << endl;
+ /* libmicrohttpd will close it. */
+ if (result_fd)
+ *result_fd = fd;
+ }
+
+ return r;
+}
// For security/portability reasons, many distro-package archives have
// a "./" in front of path names; others have nothing, others have
@@ -1614,6 +1753,7 @@ handle_buildid_r_match (bool internal_req_p,
int64_t b_mtime,
const string& b_source0,
const string& b_source1,
+ const string& section,
int *result_fd)
{
struct stat fs;
@@ -1642,6 +1782,33 @@ handle_buildid_r_match (bool internal_req_p,
break; // branch out of if "loop", to try new libarchive fetch attempt
}
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, fs.st_mtime,
+ b_source0 + ":" + b_source1,
+ section);
+ close (fd);
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for archive " << b_source0
+ << " file " << b_source1 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &fs);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno,
+ string ("fstat archive ") + b_source0 + string (" file ") + b_source1
+ + string (" section ") + section);
+ }
+ }
+
struct MHD_Response* r = MHD_create_response_from_fd (fs.st_size, fd);
if (r == 0)
{
@@ -1660,7 +1827,9 @@ handle_buildid_r_match (bool internal_req_p,
add_mhd_response_header (r, "X-DEBUGINFOD-FILE", b_source1.c_str());
add_mhd_last_modified (r, fs.st_mtime);
if (verbose > 1)
- obatched(clog) << "serving fdcache archive " << b_source0 << " file " << b_source1 << endl;
+ obatched(clog) << "serving fdcache archive " << b_source0
+ << " file " << b_source1
+ << " section=" << section << endl;
/* libmicrohttpd will close it. */
if (result_fd)
*result_fd = fd;
@@ -1791,8 +1960,36 @@ handle_buildid_r_match (bool internal_req_p,
tmppath, archive_entry_size(e),
true); // requested ones go to the front of lru
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, b_mtime,
+ b_source0 + ":" + b_source1,
+ section);
+ close (fd);
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for archive " << b_source0
+ << " file " << b_source1 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &fs);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno,
+ string ("fstat ") + b_source0 + string (" ") + section);
+ }
+ r = MHD_create_response_from_fd (fs.st_size, fd);
+ }
+ else
+ r = MHD_create_response_from_fd (archive_entry_size(e), fd);
+
inc_metric ("http_responses_total","result",archive_extension + " archive");
- r = MHD_create_response_from_fd (archive_entry_size(e), fd);
if (r == 0)
{
if (verbose)
@@ -1812,7 +2009,9 @@ handle_buildid_r_match (bool internal_req_p,
add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
add_mhd_last_modified (r, archive_entry_mtime(e));
if (verbose > 1)
- obatched(clog) << "serving archive " << b_source0 << " file " << b_source1 << endl;
+ obatched(clog) << "serving archive " << b_source0
+ << " file " << b_source1
+ << " section=" << section << endl;
/* libmicrohttpd will close it. */
if (result_fd)
*result_fd = fd;
@@ -1831,14 +2030,17 @@ handle_buildid_match (bool internal_req_p,
const string& b_stype,
const string& b_source0,
const string& b_source1,
+ const string& section,
int *result_fd)
{
try
{
if (b_stype == "F")
- return handle_buildid_f_match(internal_req_p, b_mtime, b_source0, result_fd);
+ return handle_buildid_f_match(internal_req_p, b_mtime, b_source0,
+ section, result_fd);
else if (b_stype == "R")
- return handle_buildid_r_match(internal_req_p, b_mtime, b_source0, b_source1, result_fd);
+ return handle_buildid_r_match(internal_req_p, b_mtime, b_source0,
+ b_source1, section, result_fd);
}
catch (const reportable_exception &e)
{
@@ -1913,6 +2115,7 @@ handle_buildid (MHD_Connection* conn,
if (artifacttype == "debuginfo") atype_code = "D";
else if (artifacttype == "executable") atype_code = "E";
else if (artifacttype == "source") atype_code = "S";
+ else if (artifacttype == "section") atype_code = "I";
else {
artifacttype = "invalid"; // PR28242 ensure http_resposes metrics don't propagate unclean user data
throw reportable_exception("invalid artifacttype");
@@ -1920,7 +2123,17 @@ handle_buildid (MHD_Connection* conn,
if (conn != 0)
inc_metric("http_requests_total", "type", artifacttype);
-
+
+ string section;
+ if (atype_code == "I")
+ {
+ if (suffix.size () < 2)
+ throw reportable_exception ("invalid section suffix");
+
+ // Remove leading '/'
+ section = suffix.substr(1);
+ }
+
if (atype_code == "S" && suffix == "")
throw reportable_exception("invalid source suffix");
@@ -1972,8 +2185,21 @@ handle_buildid (MHD_Connection* conn,
pp->bind(2, suffix);
pp->bind(3, canon_pathname(suffix));
}
+ else if (atype_code == "I")
+ {
+ pp = new sqlite_ps (thisdb, "mhd-query-i",
+ "select mtime, sourcetype, source0, source1, 1 as debug_p from " BUILDIDS "_query_d where buildid = ? "
+ "union all "
+ "select mtime, sourcetype, source0, source1, 0 as debug_p from " BUILDIDS "_query_e where buildid = ? "
+ "order by debug_p desc, mtime desc");
+ pp->reset();
+ pp->bind(1, buildid);
+ pp->bind(2, buildid);
+ }
unique_ptr<sqlite_ps> ps_closer(pp); // release pp if exception or return
+ bool do_upstream_section_query = true;
+
// consume all the rows
while (1)
{
@@ -1994,12 +2220,30 @@ handle_buildid (MHD_Connection* conn,
// Try accessing the located match.
// XXX: in case of multiple matches, attempt them in parallel?
auto r = handle_buildid_match (conn ? false : true,
- b_mtime, b_stype, b_source0, b_source1, result_fd);
+ b_mtime, b_stype, b_source0, b_source1,
+ section, result_fd);
if (r)
return r;
+
+ // If a debuginfo file matching BUILDID was found but didn't contain
+ // the desired section, then the section should not exist. Don't
+ // bother querying upstream servers.
+ if (!section.empty () && (sqlite3_column_int (*pp, 4) == 1))
+ {
+ struct stat st;
+
+ // For "F" sourcetype, check if the debuginfo exists. For "R"
+ // sourcetype, check if the debuginfo was interned into the fdcache.
+ if ((b_stype == "F" && (stat (b_source0.c_str (), &st) == 0))
+ || (b_stype == "R" && fdcache.probe (b_source0, b_source1)))
+ do_upstream_section_query = false;
+ }
}
pp->reset();
+ if (!do_upstream_section_query)
+ throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found");
+
// We couldn't find it in the database. Last ditch effort
// is to defer to other debuginfo servers.
@@ -2074,6 +2318,11 @@ and will not query the upstream servers");
fd = debuginfod_find_source (client,
(const unsigned char*) buildid.c_str(),
0, suffix.c_str(), NULL);
+ else if (artifacttype == "section")
+ fd = debuginfod_find_section (client,
+ (const unsigned char*) buildid.c_str(),
+ 0, section.c_str(), NULL);
+
}
else
fd = -errno; /* Set by debuginfod_begin. */
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 40b1ea00..134bb1cd 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -78,6 +78,12 @@ int debuginfod_find_source (debuginfod_client *client,
const char *filename,
char **path);
+int debuginfod_find_section (debuginfod_client *client,
+ const unsigned char *build_id,
+ int build_id_len,
+ const char *section,
+ char **path);
+
typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
void debuginfod_set_progressfn(debuginfod_client *c,
debuginfod_progressfn_t fn);
diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
index 93964167..6334373f 100644
--- a/debuginfod/libdebuginfod.map
+++ b/debuginfod/libdebuginfod.map
@@ -20,4 +20,5 @@ ELFUTILS_0.183 {
} ELFUTILS_0.179;
ELFUTILS_0.188 {
debuginfod_get_headers;
+ debuginfod_find_section;
} ELFUTILS_0.183;
diff --git a/doc/ChangeLog b/doc/ChangeLog
index b2bb4890..7d49528f 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,3 +1,10 @@
+2022-10-27 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (notrans_dist_man3_MANS): Add debuginfod_find_section.3.
+ * debuginfod_find_section.3: New file.
+ * debuginfod_find_debuginfo.3: Document debuginfod_find_section.
+ * debuginfod.8: Document section webapi.
+
2022-09-02 Frank Ch. Eigler <fche@redhat.com>
* debuginfod_find_debuginfo.3: Tweaked debuginfod_get_headers docs.
diff --git a/doc/Makefile.am b/doc/Makefile.am
index db2506fd..db5a842e 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -38,6 +38,7 @@ notrans_dist_man3_MANS += debuginfod_end.3
notrans_dist_man3_MANS += debuginfod_find_debuginfo.3
notrans_dist_man3_MANS += debuginfod_find_executable.3
notrans_dist_man3_MANS += debuginfod_find_source.3
+notrans_dist_man3_MANS += debuginfod_find_section.3
notrans_dist_man3_MANS += debuginfod_get_user_data.3
notrans_dist_man3_MANS += debuginfod_get_url.3
notrans_dist_man3_MANS += debuginfod_set_progressfn.3
diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
index 7c1dc3dd..e3964e5a 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -363,6 +363,16 @@ Note: the client should %-escape characters in /SOURCE/FILE that are
not shown as "unreserved" in section 2.3 of RFC3986. Some characters
that will be escaped include "+", "\\", "$", "!", the 'space' character,
and ";". RFC3986 includes a more comprehensive list of these characters.
+
+.SS /buildid/\fIBUILDID\fP/section\fI/SECTION\fP
+If the given buildid is known to the server, the server will attempt to
+extract the contents of an ELF/DWARF section named SECTION from the
+debuginfo file matching BUILDID. If the debuginfo file can't be found
+or the section has type SHT_NOBITS, then the server will attempt to extract
+the section from the executable matching BUILDID. If the section is
+succesfully extracted then this request results in a binary object
+of the section's contents.
+
.SS /metrics
This endpoint returns a Prometheus formatted text/plain dump of a
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index aebbec3f..d11e91b3 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -43,6 +43,12 @@ LOOKUP FUNCTIONS
.BI " int " build_id_len ","
.BI " const char *" filename ","
.BI " char ** " path ");"
+.BI "int debuginfod_find_section(debuginfod_client *" client ","
+.BI " const unsigned char *" build_id ","
+.BI " int " build_id_len ","
+.BI " const char * " section ","
+.BI " char ** " path ");"
+
OPTIONAL FUNCTIONS
@@ -98,6 +104,15 @@ accepts both forms. Specifically, debuginfod canonicalizes path names
according to RFC3986 section 5.2.4 (Remove Dot Segments), plus reducing
any \fB//\fP to \fB/\fP in the path.
+.BR debuginfod_find_section ()
+queries the debuginfod server URLs contained in
+.BR $DEBUGINFOD_URLS
+for the binary contents of an ELF/DWARF section contained in a debuginfo
+or executable file with the given \fIbuild_id\fP. \fIsection\fP should
+be the name of the desired ELF/DWARF section. If a server does not support
+section queries, debuginfod_find_section may query the server for the
+debuginfo and/or executable with \fIbuild_id\fP in order to retrieve the section.
+
If \fIpath\fP is not NULL and the query is successful, \fIpath\fP is set
to the path of the file in the cache. The caller must \fBfree\fP() this value.
@@ -228,11 +243,8 @@ void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY);
.in
.SH "SECURITY"
-.BR debuginfod_find_debuginfo (),
-.BR debuginfod_find_executable (),
-and
-.BR debuginfod_find_source ()
-\fBdo not\fP include any particular security
+.BR debuginfod_find_* ()
+functions \fBdo not\fP include any particular security
features. They trust that the binaries returned by the debuginfod(s)
are accurate. Therefore, the list of servers should include only
trustworthy ones. If accessed across HTTP rather than HTTPS, the
diff --git a/doc/debuginfod_find_section.3 b/doc/debuginfod_find_section.3
new file mode 100644
index 00000000..16279936
--- /dev/null
+++ b/doc/debuginfod_find_section.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/tests/ChangeLog b/tests/ChangeLog
index 0ea1df3d..83a86b58 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2022-10-27 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (TESTS): Add run-debuginfod-section.sh.
+ * run-debuginfod-section.sh: New test.
+
2022-10-16 Mark Wielaard <mark@klomp.org>
* dwfl-report-offline-memory.c: Include config.h first.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f680d3e1..f7074075 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -247,7 +247,8 @@ TESTS += run-debuginfod-dlopen.sh \
run-debuginfod-x-forwarded-for.sh \
run-debuginfod-response-headers.sh \
run-debuginfod-extraction-passive.sh \
- run-debuginfod-webapi-concurrency.sh
+ run-debuginfod-webapi-concurrency.sh \
+ run-debuginfod-section.sh
endif
if !OLD_LIBMICROHTTPD
# Will crash on too old libmicrohttpd
@@ -554,6 +555,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
run-debuginfod-response-headers.sh \
run-debuginfod-extraction-passive.sh \
run-debuginfod-webapi-concurrency.sh \
+ run-debuginfod-section.sh \
debuginfod-rpms/fedora30/hello2-1.0-2.src.rpm \
debuginfod-rpms/fedora30/hello2-1.0-2.x86_64.rpm \
debuginfod-rpms/fedora30/hello2-debuginfo-1.0-2.x86_64.rpm \
diff --git a/tests/run-debuginfod-section.sh b/tests/run-debuginfod-section.sh
new file mode 100755
index 00000000..7ab91718
--- /dev/null
+++ b/tests/run-debuginfod-section.sh
@@ -0,0 +1,134 @@
+#!/usr/bin/env bash
+#
+# Copyright (C) 2019-2021 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/debuginfod-subr.sh
+
+# for test case debugging, uncomment:
+set -x
+unset VALGRIND_CMD
+
+DB=${PWD}/.debuginfod_tmp.sqlite
+tempfiles $DB
+export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
+
+# Set up directories for scanning
+mkdir F
+mkdir R
+cp -rvp ${abs_srcdir}/debuginfod-rpms R
+
+# This variable is essential and ensures no time-race for claiming ports occurs
+# set base to a unique multiple of 100 not used in any other 'run-debuginfod-*' test
+base=13000
+get_ports
+
+env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE \
+ -R R -F F -p $PORT1 -d $DB -t0 -g0 -v F > vlog$PORT1 2>&1 &
+PID1=$!
+tempfiles vlog$PORT1
+errfiles vlog$PORT1
+# Server must become ready
+wait_ready $PORT1 'ready' 1
+# And initial scan should be done
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
+
+export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1
+
+# Check thread comm names
+ps -q $PID1 -e -L -o '%p %c %a' | grep groom
+ps -q $PID1 -e -L -o '%p %c %a' | grep scan
+ps -q $PID1 -e -L -o '%p %c %a' | grep traverse
+
+# We use -t0 and -g0 here to turn off time-based scanning & grooming.
+# For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process.
+
+########################################################################
+
+# Compile a simple program, strip its debuginfo and save the build-id.
+# Also move the debuginfo into another directory so that elfutils
+# cannot find it without debuginfod.
+echo "int main() { return 0; }" > ${PWD}/prog.c
+tempfiles prog.c
+
+gcc -Wl,--build-id -g -o F/prog ${PWD}/prog.c
+
+testrun ${abs_top_builddir}/src/strip -g -f F/prog.debug ${PWD}/F/prog
+BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \
+ -a F/prog | grep 'Build ID' | cut -d ' ' -f 7`
+
+kill -USR1 $PID1
+# Wait till both files are in the index.
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
+wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
+wait_ready $PORT1 'thread_busy{role="scan"}' 0
+
+########################################################################
+
+# Build-id for a file in the one of the testsuite's F31 rpms
+RPM_BUILDID=420e9e3308971f4b817cc5bf83928b41a6909d88
+
+# Download sections from files indexed with -F
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
+tempfiles vlog-find.1 vlog-find.2
+
+# Download sections from files indexed with -R
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
+
+# Verify that the downloaded files match the contents of the original sections
+objcopy F/prog.debug -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc $BUILDID.debug_info
+tempfiles ${BUILDID}.debug_info
+testrun diff -u ${BUILDID}.debug_info ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
+
+objcopy F/prog -O binary --only-section=.text ${BUILDID}.text
+tempfiles ${BUILDID}.text
+testrun diff -u ${BUILDID}.text ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
+
+# Download the original debuginfo/executable files.
+DEBUGFILE=`${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $RPM_BUILDID`
+EXECFILE=`${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_BUILDID`
+${abs_top_builddir}/debuginfod/debuginfod-find -vvv debuginfo $BUILDID
+${abs_top_builddir}/debuginfod/debuginfod-find -vvv executable $BUILDID
+
+objcopy $DEBUGFILE -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc DEBUGFILE.debug_info
+tempfiles DEBUGFILE.debug_info
+testrun diff -u DEBUGFILE.debug_info ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
+
+objcopy $EXECFILE -O binary --only-section=.text EXECFILE.text
+tempfiles EXECFILE.text
+testrun diff -u EXECFILE.text ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
+
+# Kill the server.
+kill $PID1
+wait $PID1
+PID1=0
+
+# Delete the section files from the cache.
+rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
+rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
+rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
+rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
+
+# Verify that the client can extract sections from the debuginfo or executable
+# if they're already in the cache.
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
+
+exit 0
--
2.37.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] debuginfod: Support queries for ELF/DWARF sections
2022-10-28 3:59 [PATCH v3] debuginfod: Support queries for ELF/DWARF sections Aaron Merey
@ 2022-10-30 0:29 ` Mark Wielaard
2022-11-01 4:53 ` Aaron Merey
0 siblings, 1 reply; 4+ messages in thread
From: Mark Wielaard @ 2022-10-30 0:29 UTC (permalink / raw)
To: Aaron Merey; +Cc: elfutils-devel
Hi Aaron,
On Thu, Oct 27, 2022 at 11:59:19PM -0400, Aaron Merey via Elfutils-devel wrote:
> diff --git a/NEWS b/NEWS
> index 6ebd172c..3e290ff7 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,8 @@ readelf: Add -D, --use-dynamic option.
>
> debuginfod: Add --disable-source-scan option.
>
> +debuginfod-client: Add new function debuginfod_find_section.
> +
> libdwfl: Add new function dwfl_get_debuginfod_client.
> Add new function dwfl_frame_reg.
> Add new function dwfl_report_offline_memory.
Needs reshuffling now that we have the following entry:
debuginfod-client: Add $DEBUGINFOD_HEADERS_FILE setting to supply outgoing
HTTP headers.
So they are grouped together.
> diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
> index 435cb8a6..f27d6e2e 100644
> --- a/debuginfod/Makefile.am
> +++ b/debuginfod/Makefile.am
> @@ -97,7 +97,7 @@ libdebuginfod_so_LIBS = libdebuginfod_pic.a
> if DUMMY_LIBDEBUGINFOD
> libdebuginfod_so_LDLIBS =
> else
> -libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS)
> +libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS) $(libelf)
> endif
> $(LIBDEBUGINFOD_SONAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
> $(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
OK, since we now use some elf_ calls in debuginfod-client.c
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index 2a14d9d9..0e8ca89c 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -44,6 +44,7 @@
> #include "system.h"
> #include <errno.h>
> #include <stdlib.h>
> +#include <libdwelf.h>
You want gelf.h here (libdwelf.h includes gelf/libelf.h so you don't
notice it, but if you use any dwelf_ functions you would have to pull
in libdw.so).
> /* We might be building a bootstrap dummy library, which is really simple. */
> #ifdef DUMMY_LIBDEBUGINFOD
> @@ -55,6 +56,9 @@ int debuginfod_find_executable (debuginfod_client *c, const unsigned char *b,
> int s, char **p) { return -ENOSYS; }
> int debuginfod_find_source (debuginfod_client *c, const unsigned char *b,
> int s, const char *f, char **p) { return -ENOSYS; }
> +int debuginfod_find_section (debuginfod_client *c, const unsigned char *b,
> + int s, const char *scn, char **p)
> + { return -ENOSYS; }
> void debuginfod_set_progressfn(debuginfod_client *c,
> debuginfod_progressfn_t fn) { }
> void debuginfod_set_verbose_fd(debuginfod_client *c, int fd) { }
Good.
> @@ -129,6 +133,9 @@ struct debuginfod_client
> debuginfod_end needs to terminate. */
> int default_progressfn_printed_p;
>
> + /* Indicates whether the last query was cancelled by progressfn. */
> + bool progressfn_cancel;
> +
> /* File descriptor to output any verbose messages if > 0. */
> int verbose_fd;
OK.
> @@ -536,6 +543,167 @@ header_callback (char * buffer, size_t size, size_t numitems, void * userdata)
> return numitems;
> }
>
> +/* Attempt to read an ELF/DWARF section with name SECTION from FD and write
> + it to a separate file in the debuginfod cache. If successful the absolute
> + path of the separate file containing SECTION will be stored in USR_PATH.
> + FD_PATH is the absolute path for FD.
> +
> + If the section cannot be extracted, then return a negative error code.
> + -ENOENT indicates that the parent file was able to be read but the
> + section name was not found. -EEXIST indicates that the section was
> + found but had type SHT_NOBITS. */
Nice convention.
> +int
> +extract_section (int fd, const char *section, char *fd_path, char **usr_path)
> +{
> + Elf *elf = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, NULL);
> + if (elf == NULL)
> + return -EIO;
> +
> + size_t shstrndx;
> + int rc = elf_getshdrstrndx (elf, &shstrndx);
> + if (rc < 0)
> + return -EIO;
This leaks elf.
> + /* Try to find the target section and copy the contents into a
> + separate file. */
> + Elf_Scn *scn = NULL;
> + while (true)
> + {
> + scn = elf_nextscn (elf, scn);
> + if (scn == NULL)
> + {
> + rc = -ENOENT;
> + break;
> + }
> + GElf_Shdr shdr_storage;
> + GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_storage);
> + if (shdr == NULL)
> + {
> + rc = -EIO;
> + break;
> + }
> +
> + const char *scn_name = elf_strptr (elf, shstrndx, shdr->sh_name);
> + if (scn_name == NULL)
> + {
> + rc = -EIO;
> + break;
> + }
> + if (strcmp (scn_name, section) == 0)
> + {
> + /* We found the desired section. */
> + if (shdr->sh_type == SHT_NOBITS)
> + {
> + rc = -EEXIST;
> + break;
> + }
> +
> + Elf_Data *data = NULL;
> + data = elf_getdata (scn, NULL);
Note that elf_getdata will convert the data in case of different
endian (big/little) to the host endian system. For non PROGBITS
sections, so it doesn't matter for most debug sections. But it would
matter for fetching a symbol section. It is fairly obscure, it would
only matter for someone using debuginfod to fetch a non-debug section
from a different (endian) architecture. But it seems better to just
return the raw data in that case. Just because it might be surprising
the conversion is done for you. So use elfraw_data here.
> + if (data == NULL)
> + {
> + rc = -EIO;
> + break;
> + }
> +
> + if (data->d_buf == NULL)
> + {
> + rc = -EIO;
> + break;
> + }
> +
> + /* Compute the absolute filename we'll write the section to. Replace
> + the last component of FD_PATH with the section filename. */
> + int i = strlen (fd_path);
> + while (i >= 0)
> + {
> + if (fd_path[i] == '/')
> + {
> + fd_path[i] = '\0';
> + break;
> + }
> + --i;
> + }
This seems to be dirname (fd_path), maybe that is more clear to use?
> + char *sec_path;
> + if (asprintf (&sec_path, "%s/section-%s", fd_path, section) == -1)
> + {
> + rc = -ENOMEM;
> + break;
> + }
OK. I assume this cannot clash because the original can never be called section-%s.
> + int sec_fd = open (sec_path, O_CREAT | O_EXCL | O_RDWR, S_IRUSR);
> + if (sec_fd < 0)
> + {
> + free (sec_path);
> + rc = -errno;
> + break;
> + }
> +
> + ssize_t res = write_retry (sec_fd, data->d_buf, data->d_size);
> + if (res < 0 || (size_t) res != data->d_size)
> + {
> + close (sec_fd);
> + free (sec_path);
> + rc = -EIO;
> + break;
> + }
Should this unlink the sec_path if the write_retry fails? Also this
isn't atomic. Should it write to a tempname first and then rename on
success?
> + /* Success. Update USR_PATH. */
> + if (usr_path != NULL)
> + *usr_path = sec_path;
> + else
> + free (sec_path);
> + rc = sec_fd;
> + break;
Good. sec_path is always freed on error or if usr_path is NULL. Caller
is responsible to free otherwise. Likewise for sec_fd.
> + }
> + }
> +
> + elf_end (elf);
> + return rc;
> +}
OK, in all cases elf_end is called, except for the first failure issue
noted above.
> +/* Search TARGET_CACHE_DIR for a debuginfo or executable file containing
> + an ELF/DWARF section with name SCN_NAME. If found, extract the section
> + to a separate file in TARGET_CACHE_DIR and return a file descriptor
> + for the section file. The path for this file will be stored in USR_PATH.
> + Return a negative errno if unsuccessful. */
> +
> +static int
> +cache_find_section (const char *scn_name, const char *target_cache_dir,
> + char **usr_path)
> +{
> + int fd;
> + int rc = -EEXIST;
> + char debug_path[PATH_MAX];
Not a fan of allocating PATH_MAX (4K) on the stack, but this isn't a
recursive function and we do it in other places too.
> + /* Check the debuginfo first. */
> + snprintf (debug_path, PATH_MAX, "%s/debuginfo", target_cache_dir);
> + fd = open (debug_path, O_RDONLY);
> + if (fd >= 0)
> + {
> + rc = extract_section (fd, scn_name, debug_path, usr_path);
> + close (fd);
> + }
> +
> + /* If the section type was SHT_NOBITS, check the executable. */
Ack, or the debuginfo file couldn't be found.
> + if (rc == -EEXIST)
> + {
> + char exec_path[PATH_MAX];
We don't really need two separate paths. Lets just define one path on
the stack and reuse that here.
> + snprintf (exec_path, PATH_MAX, "%s/executable", target_cache_dir);
> + fd = open (exec_path, O_RDONLY);
> +
> + if (fd >= 0)
> + {
> + rc = extract_section (fd, scn_name, exec_path, usr_path);
> + close (fd);
> + }
> + }
> +
> + return rc;
> +}
OK, rc is negative error or positive usr_path fd.
> /* Query each of the server URLs found in $DEBUGINFOD_URLS for the file
> with the specified build-id, type (debuginfo, executable or source)
> and filename. filename may be NULL. If found, return a file
> @@ -546,11 +714,13 @@ debuginfod_query_server (debuginfod_client *c,
> const unsigned char *build_id,
> int build_id_len,
> const char *type,
> - const char *filename,
> + const char *type_arg,
> char **path)
> {
Nice rename of filename/type_arg. Does need an update to the comment
at the top of the function.
> char *server_urls;
> char *urls_envvar;
> + const char *section = NULL;
> + const char *filename = NULL;
> char *cache_path = NULL;
> char *maxage_path = NULL;
> char *interval_path = NULL;
> @@ -563,6 +733,17 @@ debuginfod_query_server (debuginfod_client *c,
> int vfd = c->verbose_fd;
> int rc;
>
> + c->progressfn_cancel = false;
> +
> + if (strcmp (type, "source") == 0)
> + filename = type_arg;
> + else if (strcmp (type, "section") == 0)
> + {
> + section = type_arg;
> + if (section == NULL)
> + return -EINVAL;
> + }
OK.
> if (vfd >= 0)
> {
> dprintf (vfd, "debuginfod_find_%s ", type);
> @@ -681,6 +862,8 @@ debuginfod_query_server (debuginfod_client *c,
> PATH_MAX and truncate/collide. Oh well, that'll teach
> them! */
> }
> + else if (section != NULL)
> + strncpy (suffix, section, PATH_MAX);
> else
> suffix[0] = '\0';
OK.
> @@ -752,7 +935,10 @@ debuginfod_query_server (debuginfod_client *c,
> }
>
> xalloc_str (target_cache_dir, "%s/%s", cache_path, build_id_bytes);
> - xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
> + if (section != NULL)
> + xalloc_str (target_cache_path, "%s/%s-%s", target_cache_dir, type, suffix);
> + else
> + xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
> xalloc_str (target_cache_tmppath, "%s.XXXXXX", target_cache_path);
OK.
> /* XXX combine these */
> @@ -836,6 +1022,18 @@ debuginfod_query_server (debuginfod_client *c,
> /* Ensure old 000-permission files are not lingering in the cache. */
> unlink(target_cache_path);
>
> + if (section != NULL)
> + {
> + /* Try to extract the section from a cached file before querying
> + any servers. */
> + rc = cache_find_section (section, target_cache_dir, path);
> +
> + /* If the section was found or confirmed to not exist, then we
> + are done. */
> + if (rc >= 0 || rc == -ENOENT)
> + goto out;
> + }
OK.
> long timeout = default_timeout;
> const char* timeout_envvar = getenv(DEBUGINFOD_TIMEOUT_ENV_VAR);
> if (timeout_envvar != NULL)
> @@ -1008,6 +1206,9 @@ debuginfod_query_server (debuginfod_client *c,
> snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> build_id_bytes, type, escaped_string);
> }
> + else if (section)
> + snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> + build_id_bytes, type, section);
Does the section part need to be path escaped? Like the filename is
with curl_easy_escape? And if it is how does that interact with the
cache file name?
> else
> snprintf(data[i].url, PATH_MAX, "%s/%s/%s", server_url, build_id_bytes, type);
> if (vfd >= 0)
> @@ -1212,7 +1413,10 @@ debuginfod_query_server (debuginfod_client *c,
> }
>
> if ((*c->progressfn) (c, pa, dl_size))
> - break;
> + {
> + c->progressfn_cancel = true;
> + break;
> + }
> }
OK.
> /* Check to see if we are downloading something which exceeds maxsize, if set.*/
> @@ -1279,6 +1483,8 @@ debuginfod_query_server (debuginfod_client *c,
> /* 406 signals that the requested file was too large */
> if ( ok0 == CURLE_OK && resp_code == 406)
> rc = -EFBIG;
> + else if (section != NULL && resp_code == 503)
> + rc = -EINVAL;
> else
> rc = -ENOENT;
> break;
OK, -EINVAL means server doesn't support section fetch.
> @@ -1525,6 +1731,7 @@ debuginfod_begin (void)
> goto out1;
> }
>
> + elf_version (EV_CURRENT);
> // extra future initialization
Yeah, you need to make sure elf_version (EV_CURRENT) is called before
using any libelf function :{ But maybe do this at the start of
extract_section which is the only function that uses libelf functions.
> goto out;
> @@ -1604,6 +1811,56 @@ int debuginfod_find_source(debuginfod_client *client,
> "source", filename, path);
> }
>
> +int
> +debuginfod_find_section (debuginfod_client *client,
> + const unsigned char *build_id, int build_id_len,
> + const char *section, char **path)
> +{
> + int rc = debuginfod_query_server(client, build_id, build_id_len,
> + "section", section, path);
> + if (rc != -EINVAL)
> + return rc;
> +
> + /* The servers may have lacked support for section queries. Attempt to
> + download the debuginfo or executable containing the section in order
> + to extract it. */
This does somewhat defeat the purpose of just getting the section data
of course. So we could also just return EINVAL to the user here, so
they have to get the whole debuginfo themselves. But maybe just
pretending it worked is fine to keep the code path of the user
simpler?
> + rc = -EEXIST;
> + int fd = -1;
> + char *tmp_path = NULL;
> +
> + fd = debuginfod_find_debuginfo (client, build_id, build_id_len, &tmp_path);
> + if (client->progressfn_cancel)
> + {
> + if (fd >= 0)
> + {
> + /* This shouldn't happen, but we'll check this condition
> + just in case. */
> + close (fd);
> + free (tmp_path);
> + }
> + return -ENOENT;
> + }
> + if (fd > 0)
> + {
> + rc = extract_section (fd, section, tmp_path, path);
> + close (fd);
> + }
> +
> + if (rc == -EEXIST)
> + {
> + /* The section should be found in the executable. */
> + fd = debuginfod_find_executable (client, build_id,
> + build_id_len, &tmp_path);
> + if (fd > 0)
> + {
> + rc = extract_section (fd, section, tmp_path, path);
> + close (fd);
> + }
> + }
> +
> + free (tmp_path);
> + return rc;
> +}
OK.
> /* Add an outgoing HTTP header. */
> int debuginfod_add_http_header (debuginfod_client *client, const char* header)
> diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
> index 778fb09b..cf6e69d0 100644
> --- a/debuginfod/debuginfod-find.c
> +++ b/debuginfod/debuginfod-find.c
> @@ -190,13 +190,11 @@ main(int argc, char** argv)
> debuginfod_find_* function. If FILETYPE is "source"
> then ensure a FILENAME was also supplied as an argument. */
> if (strcmp(argv[remaining], "debuginfo") == 0)
> - rc = debuginfod_find_debuginfo(client,
> - build_id, build_id_len,
> - &cache_name);
> + rc = debuginfod_find_debuginfo(client, build_id,
> + build_id_len, &cache_name);
Putting of arguments on different lines. Not really necessary.
> else if (strcmp(argv[remaining], "executable") == 0)
> - rc = debuginfod_find_executable(client,
> - build_id, build_id_len,
> - &cache_name);
> + rc = debuginfod_find_executable(client, build_id,
> + build_id_len, &cache_name);
Likewise.
> else if (strcmp(argv[remaining], "source") == 0)
> {
> if (remaining+2 == argc || argv[remaining+2][0] != '/')
> @@ -208,6 +206,17 @@ main(int argc, char** argv)
> build_id, build_id_len,
> argv[remaining+2], &cache_name);
> }
> + else if (strcmp(argv[remaining], "section") == 0)
> + {
> + if (remaining+2 == argc)
Should this be remaining+2 >= argc ?
> + {
> + fprintf(stderr,
> + "If FILETYPE is \"section\" then a section name must be given\n");
> + return 1;
> + }
> + rc = debuginfod_find_section(client, build_id, build_id_len,
> + argv[remaining+2], &cache_name);
> + }
> else
> {
> argp_help (&argp, stderr, ARGP_HELP_USAGE, argv[0]);
The args_docs need to be updated to include the new section
arguments. For --help and --usage, which currently shows:
Usage: debuginfod-find [-v?V] [--verbose] [--help] [--usage] [--version]
debuginfo BUILDID
or: debuginfod-find [OPTION...] debuginfo PATH
or: debuginfod-find [OPTION...] executable BUILDID
or: debuginfod-find [OPTION...] executable PATH
or: debuginfod-find [OPTION...] source BUILDID /FILENAME
or: debuginfod-find [OPTION...] source PATH /FILENAME
or: debuginfod-find [OPTION...]
> diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
> index 9dc4836b..5e1dec05 100644
> --- a/debuginfod/debuginfod.cxx
> +++ b/debuginfod/debuginfod.cxx
Assuming Frank's review covered the debuginfod server part.
> diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
> index 40b1ea00..134bb1cd 100644
> --- a/debuginfod/debuginfod.h.in
> +++ b/debuginfod/debuginfod.h.in
> @@ -78,6 +78,12 @@ int debuginfod_find_source (debuginfod_client *client,
> const char *filename,
> char **path);
>
> +int debuginfod_find_section (debuginfod_client *client,
> + const unsigned char *build_id,
> + int build_id_len,
> + const char *section,
> + char **path);
> +
> typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
> void debuginfod_set_progressfn(debuginfod_client *c,
> debuginfod_progressfn_t fn);
OK.
> diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
> index 93964167..6334373f 100644
> --- a/debuginfod/libdebuginfod.map
> +++ b/debuginfod/libdebuginfod.map
> @@ -20,4 +20,5 @@ ELFUTILS_0.183 {
> } ELFUTILS_0.179;
> ELFUTILS_0.188 {
> debuginfod_get_headers;
> + debuginfod_find_section;
> } ELFUTILS_0.183;
OK.
> diff --git a/doc/ChangeLog b/doc/ChangeLog
> index b2bb4890..7d49528f 100644
> --- a/doc/ChangeLog
> +++ b/doc/ChangeLog
> @@ -1,3 +1,10 @@
> +2022-10-27 Aaron Merey <amerey@redhat.com>
> +
> + * Makefile.am (notrans_dist_man3_MANS): Add debuginfod_find_section.3.
> + * debuginfod_find_section.3: New file.
> + * debuginfod_find_debuginfo.3: Document debuginfod_find_section.
> + * debuginfod.8: Document section webapi.
> +
> 2022-09-02 Frank Ch. Eigler <fche@redhat.com>
>
> * debuginfod_find_debuginfo.3: Tweaked debuginfod_get_headers docs.
> diff --git a/doc/Makefile.am b/doc/Makefile.am
> index db2506fd..db5a842e 100644
> --- a/doc/Makefile.am
> +++ b/doc/Makefile.am
> @@ -38,6 +38,7 @@ notrans_dist_man3_MANS += debuginfod_end.3
> notrans_dist_man3_MANS += debuginfod_find_debuginfo.3
> notrans_dist_man3_MANS += debuginfod_find_executable.3
> notrans_dist_man3_MANS += debuginfod_find_source.3
> +notrans_dist_man3_MANS += debuginfod_find_section.3
> notrans_dist_man3_MANS += debuginfod_get_user_data.3
> notrans_dist_man3_MANS += debuginfod_get_url.3
> notrans_dist_man3_MANS += debuginfod_set_progressfn.3
OK.
> diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
> index 7c1dc3dd..e3964e5a 100644
> --- a/doc/debuginfod.8
> +++ b/doc/debuginfod.8
> @@ -363,6 +363,16 @@ Note: the client should %-escape characters in /SOURCE/FILE that are
> not shown as "unreserved" in section 2.3 of RFC3986. Some characters
> that will be escaped include "+", "\\", "$", "!", the 'space' character,
> and ";". RFC3986 includes a more comprehensive list of these characters.
> +
> +.SS /buildid/\fIBUILDID\fP/section\fI/SECTION\fP
> +If the given buildid is known to the server, the server will attempt to
> +extract the contents of an ELF/DWARF section named SECTION from the
> +debuginfo file matching BUILDID. If the debuginfo file can't be found
> +or the section has type SHT_NOBITS, then the server will attempt to extract
> +the section from the executable matching BUILDID. If the section is
> +succesfully extracted then this request results in a binary object
> +of the section's contents.
Maybe add something like: this is the raw section content, not an ELF file.
> .SS /metrics
>
> This endpoint returns a Prometheus formatted text/plain dump of a
> diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
> index aebbec3f..d11e91b3 100644
> --- a/doc/debuginfod_find_debuginfo.3
> +++ b/doc/debuginfod_find_debuginfo.3
> @@ -43,6 +43,12 @@ LOOKUP FUNCTIONS
> .BI " int " build_id_len ","
> .BI " const char *" filename ","
> .BI " char ** " path ");"
> +.BI "int debuginfod_find_section(debuginfod_client *" client ","
> +.BI " const unsigned char *" build_id ","
> +.BI " int " build_id_len ","
> +.BI " const char * " section ","
> +.BI " char ** " path ");"
> +
OK.
> OPTIONAL FUNCTIONS
>
> @@ -98,6 +104,15 @@ accepts both forms. Specifically, debuginfod canonicalizes path names
> according to RFC3986 section 5.2.4 (Remove Dot Segments), plus reducing
> any \fB//\fP to \fB/\fP in the path.
>
> +.BR debuginfod_find_section ()
> +queries the debuginfod server URLs contained in
> +.BR $DEBUGINFOD_URLS
> +for the binary contents of an ELF/DWARF section contained in a debuginfo
> +or executable file with the given \fIbuild_id\fP. \fIsection\fP should
> +be the name of the desired ELF/DWARF section. If a server does not support
> +section queries, debuginfod_find_section may query the server for the
> +debuginfo and/or executable with \fIbuild_id\fP in order to retrieve the section.
Maybe "to retrieve and extract the section"?
> If \fIpath\fP is not NULL and the query is successful, \fIpath\fP is set
> to the path of the file in the cache. The caller must \fBfree\fP() this value.
>
> @@ -228,11 +243,8 @@ void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY);
> .in
>
> .SH "SECURITY"
> -.BR debuginfod_find_debuginfo (),
> -.BR debuginfod_find_executable (),
> -and
> -.BR debuginfod_find_source ()
> -\fBdo not\fP include any particular security
> +.BR debuginfod_find_* ()
> +functions \fBdo not\fP include any particular security
> features. They trust that the binaries returned by the debuginfod(s)
> are accurate. Therefore, the list of servers should include only
> trustworthy ones. If accessed across HTTP rather than HTTPS, the
OK.
> diff --git a/doc/debuginfod_find_section.3 b/doc/debuginfod_find_section.3
> new file mode 100644
> index 00000000..16279936
> --- /dev/null
> +++ b/doc/debuginfod_find_section.3
> @@ -0,0 +1 @@
> +.so man3/debuginfod_find_debuginfo.3
OK.
> diff --git a/tests/ChangeLog b/tests/ChangeLog
> index 0ea1df3d..83a86b58 100644
> --- a/tests/ChangeLog
> +++ b/tests/ChangeLog
> @@ -1,3 +1,8 @@
> +2022-10-27 Aaron Merey <amerey@redhat.com>
> +
> + * Makefile.am (TESTS): Add run-debuginfod-section.sh.
> + * run-debuginfod-section.sh: New test.
> +
> 2022-10-16 Mark Wielaard <mark@klomp.org>
>
> * dwfl-report-offline-memory.c: Include config.h first.
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index f680d3e1..f7074075 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -247,7 +247,8 @@ TESTS += run-debuginfod-dlopen.sh \
> run-debuginfod-x-forwarded-for.sh \
> run-debuginfod-response-headers.sh \
> run-debuginfod-extraction-passive.sh \
> - run-debuginfod-webapi-concurrency.sh
> + run-debuginfod-webapi-concurrency.sh \
> + run-debuginfod-section.sh
> endif
> if !OLD_LIBMICROHTTPD
> # Will crash on too old libmicrohttpd
> @@ -554,6 +555,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
> run-debuginfod-response-headers.sh \
> run-debuginfod-extraction-passive.sh \
> run-debuginfod-webapi-concurrency.sh \
> + run-debuginfod-section.sh \
> debuginfod-rpms/fedora30/hello2-1.0-2.src.rpm \
> debuginfod-rpms/fedora30/hello2-1.0-2.x86_64.rpm \
> debuginfod-rpms/fedora30/hello2-debuginfo-1.0-2.x86_64.rpm \
OK.
> diff --git a/tests/run-debuginfod-section.sh b/tests/run-debuginfod-section.sh
> new file mode 100755
> index 00000000..7ab91718
> --- /dev/null
> +++ b/tests/run-debuginfod-section.sh
> @@ -0,0 +1,134 @@
> +#!/usr/bin/env bash
> +#
> +# Copyright (C) 2019-2021 Red Hat, Inc.
> +# This file is part of elfutils.
> +#
> +# This file is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# elfutils is distributed in the hope that it will be useful, but
> +# WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> +. $srcdir/debuginfod-subr.sh
> +
> +# for test case debugging, uncomment:
> +set -x
> +unset VALGRIND_CMD
> +
> +DB=${PWD}/.debuginfod_tmp.sqlite
> +tempfiles $DB
> +export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
> +
> +# Set up directories for scanning
> +mkdir F
> +mkdir R
> +cp -rvp ${abs_srcdir}/debuginfod-rpms R
> +
> +# This variable is essential and ensures no time-race for claiming ports occurs
> +# set base to a unique multiple of 100 not used in any other 'run-debuginfod-*' test
> +base=13000
> +get_ports
OK, base seems unique.
> +env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE \
> + -R R -F F -p $PORT1 -d $DB -t0 -g0 -v F > vlog$PORT1 2>&1 &
> +PID1=$!
> +tempfiles vlog$PORT1
> +errfiles vlog$PORT1
> +# Server must become ready
> +wait_ready $PORT1 'ready' 1
> +# And initial scan should be done
> +wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
> +
> +export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1
OK.
> +# Check thread comm names
> +ps -q $PID1 -e -L -o '%p %c %a' | grep groom
> +ps -q $PID1 -e -L -o '%p %c %a' | grep scan
> +ps -q $PID1 -e -L -o '%p %c %a' | grep traverse
OK, although this seems a bit of copy/paste test not really necessary here.
> +# We use -t0 and -g0 here to turn off time-based scanning & grooming.
> +# For testing purposes, we just sic SIGUSR1 / SIGUSR2 at the process.
Should this comment be earlier?
Only SIGUSR1 seems to be used below.
> +########################################################################
> +
> +# Compile a simple program, strip its debuginfo and save the build-id.
> +# Also move the debuginfo into another directory so that elfutils
> +# cannot find it without debuginfod.
> +echo "int main() { return 0; }" > ${PWD}/prog.c
> +tempfiles prog.c
> +
> +gcc -Wl,--build-id -g -o F/prog ${PWD}/prog.c
> +
> +testrun ${abs_top_builddir}/src/strip -g -f F/prog.debug ${PWD}/F/prog
> +BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \
> + -a F/prog | grep 'Build ID' | cut -d ' ' -f 7`
> +
> +kill -USR1 $PID1
> +# Wait till both files are in the index.
> +wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
> +wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
> +wait_ready $PORT1 'thread_busy{role="scan"}' 0
OK.
> +########################################################################
> +
> +# Build-id for a file in the one of the testsuite's F31 rpms
> +RPM_BUILDID=420e9e3308971f4b817cc5bf83928b41a6909d88
> +
> +# Download sections from files indexed with -F
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
OK.
> +tempfiles vlog-find.1 vlog-find.2
What are these tempfiles?
> +# Download sections from files indexed with -R
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
OK.
> +# Verify that the downloaded files match the contents of the original sections
> +objcopy F/prog.debug -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc $BUILDID.debug_info
> +tempfiles ${BUILDID}.debug_info
> +testrun diff -u ${BUILDID}.debug_info ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
Best to define tempfiles before creating them.
Since these are binary files maybe better to use cmp?
You don't need testrun for ordinary binaries.
> +objcopy F/prog -O binary --only-section=.text ${BUILDID}.text
> +tempfiles ${BUILDID}.text
> +testrun diff -u ${BUILDID}.text ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
Likewise.
> +# Download the original debuginfo/executable files.
> +DEBUGFILE=`${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $RPM_BUILDID`
> +EXECFILE=`${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_BUILDID`
> +${abs_top_builddir}/debuginfod/debuginfod-find -vvv debuginfo $BUILDID
> +${abs_top_builddir}/debuginfod/debuginfod-find -vvv executable $BUILDID
You do need testrun here, or use env LD_LIBRARY_PATH=$ldpath
DEBUGFILE=`env LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $RPM_BUILDID`
EXECFILE=`env LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_BUILDID`
testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv debuginfo $BUILDID
testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv executable $BUILDID
Otherwise debuginfod-find will run against the system elfutils
libraries, and we want to test the just build ones.
> +objcopy $DEBUGFILE -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc DEBUGFILE.debug_info
> +tempfiles DEBUGFILE.debug_info
> +testrun diff -u DEBUGFILE.debug_info ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
> +
> +objcopy $EXECFILE -O binary --only-section=.text EXECFILE.text
> +tempfiles EXECFILE.text
> +testrun diff -u EXECFILE.text ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
Same comments as above. But also unfortunately binutils objcopy might
not work on arches that aren't x86_64. On my debian arm64 setup it
says:
objcopy: Unable to recognise the format of the input file `/home/mark/src/elfutils/tests/test-279479/.client_cache/420e9e3308971f4b817cc5bf83928b41a6909d88/debuginfo'
This is because the rpm binaries are x86_64. So wrap this in something like:
if test "$(arch)" = "x86_64"; then
tempfiles DEBUGFILE.debug_info
objcopy $DEBUGFILE -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc DEBUGFILE.debug_info
cmp DEBUGFILE.debug_info ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
tempfiles EXECFILE.text
objcopy $EXECFILE -O binary --only-section=.text EXECFILE.text
cmp EXECFILE.text ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
fi
> +# Kill the server.
> +kill $PID1
> +wait $PID1
> +PID1=0
> +
> +# Delete the section files from the cache.
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
> +rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
> +
> +# Verify that the client can extract sections from the debuginfo or executable
> +# if they're already in the cache.
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
> +testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
> +
> +exit 0
OK.
Cheers,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v3] debuginfod: Support queries for ELF/DWARF sections
2022-10-30 0:29 ` Mark Wielaard
@ 2022-11-01 4:53 ` Aaron Merey
2022-11-01 20:29 ` Mark Wielaard
0 siblings, 1 reply; 4+ messages in thread
From: Aaron Merey @ 2022-11-01 4:53 UTC (permalink / raw)
To: mark; +Cc: elfutils-devel, Aaron Merey
Hi Mark,
Thanks again for the detailed review. I fixed the issues you pointed out.
On Sat, Oct 29, 2022 at 8:29 PM Mark Wielaard <mark@klomp.org> wrote:
> > @@ -1008,6 +1206,9 @@ debuginfod_query_server (debuginfod_client *c,
> > snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> > build_id_bytes, type, escaped_string);
> > }
> > + else if (section)
> > + snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> > + build_id_bytes, type, section);
>
> Does the section part need to be path escaped? Like the filename is
> with curl_easy_escape? And if it is how does that interact with the
> cache file name?
I assumed that section names would not contain '/'. However I noticed
that if we call debuginfod_find_section with a section name containing
'/', it will return -EINVAL. This triggers the downloading of debuginfo
in order to attempt client-side extraction. This is a waste so I changed
the client to path-escape the section name for caching purposes just like
the source query filename.
> > + /* The servers may have lacked support for section queries. Attempt to
> > + download the debuginfo or executable containing the section in order
> > + to extract it. */
>
> This does somewhat defeat the purpose of just getting the section data
> of course. So we could also just return EINVAL to the user here, so
> they have to get the whole debuginfo themselves. But maybe just
> pretending it worked is fine to keep the code path of the user
> simpler?
This keeps the code path simpler for the user and like Frank mentioned
it enables an intermediate server to extract the section and provide
it to the client. This server will also cache the debuginfo/executable,
possibly speeding up future requests.
> > +tempfiles vlog-find.1 vlog-find.2
>
> What are these tempfiles?
This was left over from v2 of this patch and is now removed.
Aaron
---
Add new function debuginfod_find_section which queries debuginfod
servers for the raw binary contents of the specified ELF/DWARF section
in a file matching the given build-id.
Extend the server webapi to support section queries. Section query
URLS have the following format: /buildid/BUILDID/section/SECTION
The server will attempt to extract the section from a debuginfo file
matching the given build-id. If the debuginfo file cannot be found
or the section has type SHT_NOBITS, the server will attempt to extract
the section from the executable file matching the build-id.
If the server is built without section query support, the client will
attempt to download the debuginfo matching the build-id and extract the
section. If the debuginfo file cannot be found or the section has type
SHT_NOBITS, the server will attempt to download the executable file
matching the build-id and extract the section.
Signed-off-by: Aaron Merey <amerey@redhat.com>
---
ChangeLog | 4 +
NEWS | 2 +-
debuginfod/ChangeLog | 22 ++
debuginfod/Makefile.am | 2 +-
debuginfod/debuginfod-client.c | 363 +++++++++++++++++++++++++++---
debuginfod/debuginfod-find.c | 15 +-
debuginfod/debuginfod.cxx | 383 ++++++++++++++++++++++++++------
debuginfod/debuginfod.h.in | 6 +
debuginfod/libdebuginfod.map | 1 +
doc/ChangeLog | 7 +
doc/Makefile.am | 1 +
doc/debuginfod.8 | 11 +
doc/debuginfod_find_debuginfo.3 | 23 +-
doc/debuginfod_find_section.3 | 1 +
tests/ChangeLog | 5 +
tests/Makefile.am | 4 +-
tests/run-debuginfod-section.sh | 135 +++++++++++
17 files changed, 882 insertions(+), 103 deletions(-)
create mode 100644 doc/debuginfod_find_section.3
create mode 100755 tests/run-debuginfod-section.sh
diff --git a/ChangeLog b/ChangeLog
index 7bbb2c0f..9179dcea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2022-10-31 Aaron Merey <amerey@redhat.com>
+
+ * NEWS: Add debuginfod_find_section.
+
2022-10-20 Mark Wielaard <mark@klomp.org>
* Makefile.am (rpm): Remove --sign.
diff --git a/NEWS b/NEWS
index 3df652e3..9369f809 100644
--- a/NEWS
+++ b/NEWS
@@ -3,7 +3,7 @@ Version 0.188 some time after 0.187
readelf: Add -D, --use-dynamic option.
debuginfod-client: Add $DEBUGINFOD_HEADERS_FILE setting to supply outgoing
- HTTP headers.
+ HTTP headers. Add new function debuginfod_find_section.
debuginfod: Add --disable-source-scan option.
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 1df903fe..92a880f8 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,25 @@
+2022-10-31 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (libdebuginfod_so_LDLIBS): Add libelf.
+ * debuginfod-client.c (debuginfod_find_section): New function.
+ (path_escape): New function.
+ (extract_section): New function.
+ (cache_find_section): New function.
+ (debuginfod_query_server): Add support for section queries.
+ * debuginfod-find.c (main): Add support for section queries.
+ * debuginfod.cxx (extract_section): New function.
+ (handle_buildid_f_match): Add section parameter. When non-empty,
+ try to create response from section contents.
+ (handle_buildid_r_match): Add section parameter. When non-empty,
+ try to create response from section contents.
+ (handle_buildid_match): Add section parameter. Pass to
+ handle_buildid_{f,r}_match.
+ (handle_buildid): Handle section name when artifacttype is set to
+ "section". Query upstream servers via debuginfod_find_section
+ when necessary.
+ (debuginfod.h.in): Add declaration for debuginfod_find_section.
+ (libdebuginfod.map): Add debuginfod_find_section.
+
2022-10-18 Daniel Thornburgh <dthorn@google.com>
* debuginfod-client.c (debuginfod_query_server): Add DEBUGINFOD_HEADERS_FILE
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 435cb8a6..f27d6e2e 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -97,7 +97,7 @@ libdebuginfod_so_LIBS = libdebuginfod_pic.a
if DUMMY_LIBDEBUGINFOD
libdebuginfod_so_LDLIBS =
else
-libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS)
+libdebuginfod_so_LDLIBS = -lpthread $(libcurl_LIBS) $(fts_LIBS) $(libelf)
endif
$(LIBDEBUGINFOD_SONAME): $(srcdir)/libdebuginfod.map $(libdebuginfod_so_LIBS)
$(AM_V_CCLD)$(LINK) $(dso_LDFLAGS) -o $@ \
diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
index 716cb769..d097ca49 100644
--- a/debuginfod/debuginfod-client.c
+++ b/debuginfod/debuginfod-client.c
@@ -45,6 +45,7 @@
#include <ctype.h>
#include <errno.h>
#include <stdlib.h>
+#include <gelf.h>
/* We might be building a bootstrap dummy library, which is really simple. */
#ifdef DUMMY_LIBDEBUGINFOD
@@ -56,6 +57,9 @@ int debuginfod_find_executable (debuginfod_client *c, const unsigned char *b,
int s, char **p) { return -ENOSYS; }
int debuginfod_find_source (debuginfod_client *c, const unsigned char *b,
int s, const char *f, char **p) { return -ENOSYS; }
+int debuginfod_find_section (debuginfod_client *c, const unsigned char *b,
+ int s, const char *scn, char **p)
+ { return -ENOSYS; }
void debuginfod_set_progressfn(debuginfod_client *c,
debuginfod_progressfn_t fn) { }
void debuginfod_set_verbose_fd(debuginfod_client *c, int fd) { }
@@ -130,6 +134,9 @@ struct debuginfod_client
debuginfod_end needs to terminate. */
int default_progressfn_printed_p;
+ /* Indicates whether the last query was cancelled by progressfn. */
+ bool progressfn_cancel;
+
/* File descriptor to output any verbose messages if > 0. */
int verbose_fd;
@@ -576,21 +583,257 @@ header_callback (char * buffer, size_t size, size_t numitems, void * userdata)
return numitems;
}
+/* Copy SRC to DEST, s,/,#,g */
+
+static void
+path_escape (const char *src, char *dest)
+{
+ unsigned q = 0;
+
+ for (unsigned fi=0; q < PATH_MAX-2; fi++) /* -2, escape is 2 chars. */
+ switch (src[fi])
+ {
+ case '\0':
+ dest[q] = '\0';
+ q = PATH_MAX-1; /* escape for loop too */
+ break;
+ case '/': /* escape / to prevent dir escape */
+ dest[q++]='#';
+ dest[q++]='#';
+ break;
+ case '#': /* escape # to prevent /# vs #/ collisions */
+ dest[q++]='#';
+ dest[q++]='_';
+ break;
+ default:
+ dest[q++]=src[fi];
+ }
+
+ dest[q] = '\0';
+}
+
+/* Attempt to read an ELF/DWARF section with name SECTION from FD and write
+ it to a separate file in the debuginfod cache. If successful the absolute
+ path of the separate file containing SECTION will be stored in USR_PATH.
+ FD_PATH is the absolute path for FD.
+
+ If the section cannot be extracted, then return a negative error code.
+ -ENOENT indicates that the parent file was able to be read but the
+ section name was not found. -EEXIST indicates that the section was
+ found but had type SHT_NOBITS. */
+
+int
+extract_section (int fd, const char *section, char *fd_path, char **usr_path)
+{
+ elf_version (EV_CURRENT);
+
+ Elf *elf = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, NULL);
+ if (elf == NULL)
+ return -EIO;
+
+ size_t shstrndx;
+ int rc = elf_getshdrstrndx (elf, &shstrndx);
+ if (rc < 0)
+ {
+ rc = -EIO;
+ goto out;
+ }
+
+ int sec_fd = -1;
+ char *escaped_name = NULL;
+ char *sec_path_tmp = NULL;
+ Elf_Scn *scn = NULL;
+
+ /* Try to find the target section and copy the contents into a
+ separate file. */
+ while (true)
+ {
+ scn = elf_nextscn (elf, scn);
+ if (scn == NULL)
+ {
+ rc = -ENOENT;
+ goto out;
+ }
+ GElf_Shdr shdr_storage;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_storage);
+ if (shdr == NULL)
+ {
+ rc = -EIO;
+ goto out;
+ }
+
+ const char *scn_name = elf_strptr (elf, shstrndx, shdr->sh_name);
+ if (scn_name == NULL)
+ {
+ rc = -EIO;
+ goto out;
+ }
+ if (strcmp (scn_name, section) == 0)
+ {
+ /* We found the desired section. */
+ if (shdr->sh_type == SHT_NOBITS)
+ {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ Elf_Data *data = NULL;
+ data = elf_rawdata (scn, NULL);
+ if (data == NULL)
+ {
+ rc = -EIO;
+ goto out;
+ }
+
+ if (data->d_buf == NULL)
+ {
+ rc = -EIO;
+ goto out;
+ }
+
+ /* Compute the absolute filename we'll write the section to.
+ Replace the last component of FD_PATH with the path-escaped
+ section filename. */
+ int i = strlen (fd_path);
+ while (i >= 0)
+ {
+ if (fd_path[i] == '/')
+ {
+ fd_path[i] = '\0';
+ break;
+ }
+ --i;
+ }
+
+ escaped_name = malloc (strlen (section) * 2 + 1);
+ if (escaped_name == NULL)
+ {
+ rc = -ENOMEM;
+ goto out;
+ }
+ path_escape (section, escaped_name);
+
+ rc = asprintf (&sec_path_tmp, "%s/section-%s.XXXXXX",
+ fd_path, escaped_name);
+ if (rc == -1)
+ {
+ rc = -ENOMEM;
+ goto out1;
+ }
+
+ sec_fd = mkstemp (sec_path_tmp);
+ if (sec_fd < 0)
+ {
+ rc = -EIO;
+ goto out2;
+ }
+
+ ssize_t res = write_retry (sec_fd, data->d_buf, data->d_size);
+ if (res < 0 || (size_t) res != data->d_size)
+ {
+ rc = -EIO;
+ goto out3;
+ }
+
+ /* Success. Rename tmp file and update USR_PATH. */
+ char *sec_path;
+ if (asprintf (&sec_path, "%s/section-%s", fd_path, section) == -1)
+ {
+ rc = -ENOMEM;
+ goto out3;
+ }
+
+ rc = rename (sec_path_tmp, sec_path);
+ if (rc < 0)
+ {
+ free (sec_path);
+ rc = -EIO;
+ goto out3;
+ }
+
+ if (usr_path != NULL)
+ *usr_path = sec_path;
+ else
+ free (sec_path);
+ rc = sec_fd;
+ goto out2;
+ }
+ }
+
+out3:
+ close (sec_fd);
+ unlink (sec_path_tmp);
+
+out2:
+ free (sec_path_tmp);
+
+out1:
+ free (escaped_name);
+
+out:
+ elf_end (elf);
+ return rc;
+}
+
+/* Search TARGET_CACHE_DIR for a debuginfo or executable file containing
+ an ELF/DWARF section with name SCN_NAME. If found, extract the section
+ to a separate file in TARGET_CACHE_DIR and return a file descriptor
+ for the section file. The path for this file will be stored in USR_PATH.
+ Return a negative errno if unsuccessful. */
+
+static int
+cache_find_section (const char *scn_name, const char *target_cache_dir,
+ char **usr_path)
+{
+ int fd;
+ int rc = -EEXIST;
+ char parent_path[PATH_MAX];
+
+ /* Check the debuginfo first. */
+ snprintf (parent_path, PATH_MAX, "%s/debuginfo", target_cache_dir);
+ fd = open (parent_path, O_RDONLY);
+ if (fd >= 0)
+ {
+ rc = extract_section (fd, scn_name, parent_path, usr_path);
+ close (fd);
+ }
+
+ /* If the debuginfo file couldn't be found or the section type was
+ SHT_NOBITS, check the executable. */
+ if (rc == -EEXIST)
+ {
+ snprintf (parent_path, PATH_MAX, "%s/executable", target_cache_dir);
+ fd = open (parent_path, O_RDONLY);
+
+ if (fd >= 0)
+ {
+ rc = extract_section (fd, scn_name, parent_path, usr_path);
+ close (fd);
+ }
+ }
+
+ return rc;
+}
+
/* Query each of the server URLs found in $DEBUGINFOD_URLS for the file
- with the specified build-id, type (debuginfo, executable or source)
- and filename. filename may be NULL. If found, return a file
- descriptor for the target, otherwise return an error code.
+ with the specified build-id and type (debuginfo, executable, source or
+ section). If type is source, then type_arg should be a filename. If
+ type is section, then type_arg should be the name of an ELF/DWARF
+ section. Otherwise type_arg may be NULL. Return a file descriptor
+ for the target if successful, otherwise return an error code.
*/
static int
debuginfod_query_server (debuginfod_client *c,
const unsigned char *build_id,
int build_id_len,
const char *type,
- const char *filename,
+ const char *type_arg,
char **path)
{
char *server_urls;
char *urls_envvar;
+ const char *section = NULL;
+ const char *filename = NULL;
char *cache_path = NULL;
char *maxage_path = NULL;
char *interval_path = NULL;
@@ -603,6 +846,17 @@ debuginfod_query_server (debuginfod_client *c,
int vfd = c->verbose_fd;
int rc;
+ c->progressfn_cancel = false;
+
+ if (strcmp (type, "source") == 0)
+ filename = type_arg;
+ else if (strcmp (type, "section") == 0)
+ {
+ section = type_arg;
+ if (section == NULL)
+ return -EINVAL;
+ }
+
if (vfd >= 0)
{
dprintf (vfd, "debuginfod_find_%s ", type);
@@ -701,31 +955,13 @@ debuginfod_query_server (debuginfod_client *c,
goto out;
}
- /* copy the filename to suffix, s,/,#,g */
- unsigned q = 0;
- for (unsigned fi=0; q < PATH_MAX-2; fi++) /* -2, escape is 2 chars. */
- switch (filename[fi])
- {
- case '\0':
- suffix[q] = '\0';
- q = PATH_MAX-1; /* escape for loop too */
- break;
- case '/': /* escape / to prevent dir escape */
- suffix[q++]='#';
- suffix[q++]='#';
- break;
- case '#': /* escape # to prevent /# vs #/ collisions */
- suffix[q++]='#';
- suffix[q++]='_';
- break;
- default:
- suffix[q++]=filename[fi];
- }
- suffix[q] = '\0';
+ path_escape (filename, suffix);
/* If the DWARF filenames are super long, this could exceed
PATH_MAX and truncate/collide. Oh well, that'll teach
them! */
}
+ else if (section != NULL)
+ path_escape (section, suffix);
else
suffix[0] = '\0';
@@ -797,7 +1033,10 @@ debuginfod_query_server (debuginfod_client *c,
}
xalloc_str (target_cache_dir, "%s/%s", cache_path, build_id_bytes);
- xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
+ if (section != NULL)
+ xalloc_str (target_cache_path, "%s/%s-%s", target_cache_dir, type, suffix);
+ else
+ xalloc_str (target_cache_path, "%s/%s%s", target_cache_dir, type, suffix);
xalloc_str (target_cache_tmppath, "%s.XXXXXX", target_cache_path);
/* XXX combine these */
@@ -881,6 +1120,18 @@ debuginfod_query_server (debuginfod_client *c,
/* Ensure old 000-permission files are not lingering in the cache. */
unlink(target_cache_path);
+ if (section != NULL)
+ {
+ /* Try to extract the section from a cached file before querying
+ any servers. */
+ rc = cache_find_section (section, target_cache_dir, path);
+
+ /* If the section was found or confirmed to not exist, then we
+ are done. */
+ if (rc >= 0 || rc == -ENOENT)
+ goto out;
+ }
+
long timeout = default_timeout;
const char* timeout_envvar = getenv(DEBUGINFOD_TIMEOUT_ENV_VAR);
if (timeout_envvar != NULL)
@@ -1053,6 +1304,9 @@ debuginfod_query_server (debuginfod_client *c,
snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
build_id_bytes, type, escaped_string);
}
+ else if (section)
+ snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
+ build_id_bytes, type, section);
else
snprintf(data[i].url, PATH_MAX, "%s/%s/%s", server_url, build_id_bytes, type);
if (vfd >= 0)
@@ -1257,7 +1511,10 @@ debuginfod_query_server (debuginfod_client *c,
}
if ((*c->progressfn) (c, pa, dl_size))
- break;
+ {
+ c->progressfn_cancel = true;
+ break;
+ }
}
/* Check to see if we are downloading something which exceeds maxsize, if set.*/
@@ -1324,6 +1581,8 @@ debuginfod_query_server (debuginfod_client *c,
/* 406 signals that the requested file was too large */
if ( ok0 == CURLE_OK && resp_code == 406)
rc = -EFBIG;
+ else if (section != NULL && resp_code == 503)
+ rc = -EINVAL;
else
rc = -ENOENT;
break;
@@ -1649,6 +1908,56 @@ int debuginfod_find_source(debuginfod_client *client,
"source", filename, path);
}
+int
+debuginfod_find_section (debuginfod_client *client,
+ const unsigned char *build_id, int build_id_len,
+ const char *section, char **path)
+{
+ int rc = debuginfod_query_server(client, build_id, build_id_len,
+ "section", section, path);
+ if (rc != -EINVAL)
+ return rc;
+
+ /* The servers may have lacked support for section queries. Attempt to
+ download the debuginfo or executable containing the section in order
+ to extract it. */
+ rc = -EEXIST;
+ int fd = -1;
+ char *tmp_path = NULL;
+
+ fd = debuginfod_find_debuginfo (client, build_id, build_id_len, &tmp_path);
+ if (client->progressfn_cancel)
+ {
+ if (fd >= 0)
+ {
+ /* This shouldn't happen, but we'll check this condition
+ just in case. */
+ close (fd);
+ free (tmp_path);
+ }
+ return -ENOENT;
+ }
+ if (fd > 0)
+ {
+ rc = extract_section (fd, section, tmp_path, path);
+ close (fd);
+ }
+
+ if (rc == -EEXIST)
+ {
+ /* The section should be found in the executable. */
+ fd = debuginfod_find_executable (client, build_id,
+ build_id_len, &tmp_path);
+ if (fd > 0)
+ {
+ rc = extract_section (fd, section, tmp_path, path);
+ close (fd);
+ }
+ }
+
+ free (tmp_path);
+ return rc;
+}
/* Add an outgoing HTTP header. */
int debuginfod_add_http_header (debuginfod_client *client, const char* header)
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 778fb09b..30731098 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -48,7 +48,9 @@ static const char args_doc[] = N_("debuginfo BUILDID\n"
"executable BUILDID\n"
"executable PATH\n"
"source BUILDID /FILENAME\n"
- "source PATH /FILENAME\n");
+ "source PATH /FILENAME\n"
+ "section BUILDID SECTION-NAME\n"
+ "section PATH SECTION-NAME\n");
/* Definitions of arguments for argp functions. */
@@ -208,6 +210,17 @@ main(int argc, char** argv)
build_id, build_id_len,
argv[remaining+2], &cache_name);
}
+ else if (strcmp(argv[remaining], "section") == 0)
+ {
+ if (remaining+2 >= argc)
+ {
+ fprintf(stderr,
+ "If FILETYPE is \"section\" then a section name must be given\n");
+ return 1;
+ }
+ rc = debuginfod_find_section(client, build_id, build_id_len,
+ argv[remaining+2], &cache_name);
+ }
else
{
argp_help (&argp, stderr, ARGP_HELP_USAGE, argv[0]);
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index 9dc4836b..f46da6ef 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -1136,66 +1136,6 @@ add_mhd_last_modified (struct MHD_Response *resp, time_t mtime)
add_mhd_response_header (resp, "Cache-Control", "public");
}
-
-
-static struct MHD_Response*
-handle_buildid_f_match (bool internal_req_t,
- int64_t b_mtime,
- const string& b_source0,
- int *result_fd)
-{
- (void) internal_req_t; // ignored
- int fd = open(b_source0.c_str(), O_RDONLY);
- if (fd < 0)
- throw libc_exception (errno, string("open ") + b_source0);
-
- // NB: use manual close(2) in error case instead of defer_dtor, because
- // in the normal case, we want to hand the fd over to libmicrohttpd for
- // file transfer.
-
- struct stat s;
- int rc = fstat(fd, &s);
- if (rc < 0)
- {
- close(fd);
- throw libc_exception (errno, string("fstat ") + b_source0);
- }
-
- if ((int64_t) s.st_mtime != b_mtime)
- {
- if (verbose)
- obatched(clog) << "mtime mismatch for " << b_source0 << endl;
- close(fd);
- return 0;
- }
-
- inc_metric ("http_responses_total","result","file");
- struct MHD_Response* r = MHD_create_response_from_fd ((uint64_t) s.st_size, fd);
- if (r == 0)
- {
- if (verbose)
- obatched(clog) << "cannot create fd-response for " << b_source0 << endl;
- close(fd);
- }
- else
- {
- std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length());
- add_mhd_response_header (r, "Content-Type", "application/octet-stream");
- add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
- to_string(s.st_size).c_str());
- add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
- add_mhd_last_modified (r, s.st_mtime);
- if (verbose > 1)
- obatched(clog) << "serving file " << b_source0 << endl;
- /* libmicrohttpd will close it. */
- if (result_fd)
- *result_fd = fd;
- }
-
- return r;
-}
-
-
// quote all questionable characters of str for safe passage through a sh -c expansion.
static string
shell_escape(const string& str)
@@ -1589,6 +1529,205 @@ public:
};
static libarchive_fdcache fdcache;
+/* Search ELF_FD for an ELF/DWARF section with name SECTION.
+ If found copy the section to a temporary file and return
+ its file descriptor, otherwise return -1.
+
+ The temporary file's mtime will be set to PARENT_MTIME.
+ B_SOURCE should be a description of the parent file suitable
+ for printing to the log. */
+
+static int
+extract_section (int elf_fd, int64_t parent_mtime,
+ const string& b_source, const string& section)
+{
+ /* Search the fdcache. */
+ struct stat fs;
+ int fd = fdcache.lookup (b_source, section);
+ if (fd >= 0)
+ {
+ if (fstat (fd, &fs) != 0)
+ {
+ if (verbose)
+ obatched (clog) << "cannot fstate fdcache "
+ << b_source << " " << section << endl;
+ close (fd);
+ return -1;
+ }
+ if ((int64_t) fs.st_mtime != parent_mtime)
+ {
+ if (verbose)
+ obatched(clog) << "mtime mismatch for "
+ << b_source << " " << section << endl;
+ close (fd);
+ return -1;
+ }
+ /* Success. */
+ return fd;
+ }
+
+ Elf *elf = elf_begin (elf_fd, ELF_C_READ_MMAP_PRIVATE, NULL);
+ if (elf == NULL)
+ return -1;
+
+ /* Try to find the section and copy the contents into a separate file. */
+ try
+ {
+ size_t shstrndx;
+ int rc = elf_getshdrstrndx (elf, &shstrndx);
+ if (rc < 0)
+ throw elfutils_exception (rc, "getshdrstrndx");
+
+ Elf_Scn *scn = NULL;
+ while (true)
+ {
+ scn = elf_nextscn (elf, scn);
+ if (scn == NULL)
+ break;
+ GElf_Shdr shdr_storage;
+ GElf_Shdr *shdr = gelf_getshdr (scn, &shdr_storage);
+ if (shdr == NULL)
+ break;
+
+ const char *scn_name = elf_strptr (elf, shstrndx, shdr->sh_name);
+ if (scn_name == NULL)
+ break;
+ if (scn_name == section)
+ {
+ Elf_Data *data = NULL;
+
+ /* We found the desired section. */
+ data = elf_rawdata (scn, NULL);
+ if (data == NULL)
+ throw elfutils_exception (elf_errno (), "elfraw_data");
+ if (data->d_buf == NULL)
+ {
+ obatched(clog) << "section " << section
+ << " is empty" << endl;
+ break;
+ }
+
+ /* Create temporary file containing the section. */
+ char *tmppath = NULL;
+ rc = asprintf (&tmppath, "%s/debuginfod.XXXXXX", tmpdir.c_str());
+ if (rc < 0)
+ throw libc_exception (ENOMEM, "cannot allocate tmppath");
+ defer_dtor<void*,void> tmmpath_freer (tmppath, free);
+ fd = mkstemp (tmppath);
+ if (fd < 0)
+ throw libc_exception (errno, "cannot create temporary file");
+ ssize_t res = write_retry (fd, data->d_buf, data->d_size);
+ if (res < 0 || (size_t) res != data->d_size)
+ throw libc_exception (errno, "cannot write to temporary file");
+
+ /* Set mtime to be the same as the parent file's mtime. */
+ struct timeval tvs[2];
+ if (fstat (elf_fd, &fs) != 0)
+ throw libc_exception (errno, "cannot fstat file");
+
+ tvs[0].tv_sec = tvs[1].tv_sec = fs.st_mtime;
+ tvs[0].tv_usec = tvs[1].tv_usec = 0;
+ (void) futimes (fd, tvs);
+
+ /* Add to fdcache. */
+ fdcache.intern (b_source, section, tmppath, data->d_size, true);
+ break;
+ }
+ }
+ }
+ catch (const reportable_exception &e)
+ {
+ e.report (clog);
+ close (fd);
+ fd = -1;
+ }
+
+ elf_end (elf);
+ return fd;
+}
+
+static struct MHD_Response*
+handle_buildid_f_match (bool internal_req_t,
+ int64_t b_mtime,
+ const string& b_source0,
+ const string& section,
+ int *result_fd)
+{
+ (void) internal_req_t; // ignored
+ int fd = open(b_source0.c_str(), O_RDONLY);
+ if (fd < 0)
+ throw libc_exception (errno, string("open ") + b_source0);
+
+ // NB: use manual close(2) in error case instead of defer_dtor, because
+ // in the normal case, we want to hand the fd over to libmicrohttpd for
+ // file transfer.
+
+ struct stat s;
+ int rc = fstat(fd, &s);
+ if (rc < 0)
+ {
+ close(fd);
+ throw libc_exception (errno, string("fstat ") + b_source0);
+ }
+
+ if ((int64_t) s.st_mtime != b_mtime)
+ {
+ if (verbose)
+ obatched(clog) << "mtime mismatch for " << b_source0 << endl;
+ close(fd);
+ return 0;
+ }
+
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, s.st_mtime, b_source0, section);
+ close (fd);
+
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for " << b_source0 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &s);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno, string ("fstat ") + b_source0
+ + string (" ") + section);
+ }
+ }
+
+ struct MHD_Response* r = MHD_create_response_from_fd ((uint64_t) s.st_size, fd);
+ inc_metric ("http_responses_total","result","file");
+ if (r == 0)
+ {
+ if (verbose)
+ obatched(clog) << "cannot create fd-response for " << b_source0
+ << " section=" << section << endl;
+ close(fd);
+ }
+ else
+ {
+ std::string file = b_source0.substr(b_source0.find_last_of("/")+1, b_source0.length());
+ add_mhd_response_header (r, "Content-Type", "application/octet-stream");
+ add_mhd_response_header (r, "X-DEBUGINFOD-SIZE",
+ to_string(s.st_size).c_str());
+ add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
+ add_mhd_last_modified (r, s.st_mtime);
+ if (verbose > 1)
+ obatched(clog) << "serving file " << b_source0 << " section=" << section << endl;
+ /* libmicrohttpd will close it. */
+ if (result_fd)
+ *result_fd = fd;
+ }
+
+ return r;
+}
// For security/portability reasons, many distro-package archives have
// a "./" in front of path names; others have nothing, others have
@@ -1614,6 +1753,7 @@ handle_buildid_r_match (bool internal_req_p,
int64_t b_mtime,
const string& b_source0,
const string& b_source1,
+ const string& section,
int *result_fd)
{
struct stat fs;
@@ -1642,6 +1782,33 @@ handle_buildid_r_match (bool internal_req_p,
break; // branch out of if "loop", to try new libarchive fetch attempt
}
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, fs.st_mtime,
+ b_source0 + ":" + b_source1,
+ section);
+ close (fd);
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for archive " << b_source0
+ << " file " << b_source1 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &fs);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno,
+ string ("fstat archive ") + b_source0 + string (" file ") + b_source1
+ + string (" section ") + section);
+ }
+ }
+
struct MHD_Response* r = MHD_create_response_from_fd (fs.st_size, fd);
if (r == 0)
{
@@ -1660,7 +1827,9 @@ handle_buildid_r_match (bool internal_req_p,
add_mhd_response_header (r, "X-DEBUGINFOD-FILE", b_source1.c_str());
add_mhd_last_modified (r, fs.st_mtime);
if (verbose > 1)
- obatched(clog) << "serving fdcache archive " << b_source0 << " file " << b_source1 << endl;
+ obatched(clog) << "serving fdcache archive " << b_source0
+ << " file " << b_source1
+ << " section=" << section << endl;
/* libmicrohttpd will close it. */
if (result_fd)
*result_fd = fd;
@@ -1791,8 +1960,36 @@ handle_buildid_r_match (bool internal_req_p,
tmppath, archive_entry_size(e),
true); // requested ones go to the front of lru
+ if (!section.empty ())
+ {
+ int scn_fd = extract_section (fd, b_mtime,
+ b_source0 + ":" + b_source1,
+ section);
+ close (fd);
+ if (scn_fd >= 0)
+ fd = scn_fd;
+ else
+ {
+ if (verbose)
+ obatched (clog) << "cannot find section " << section
+ << " for archive " << b_source0
+ << " file " << b_source1 << endl;
+ return 0;
+ }
+
+ rc = fstat(fd, &fs);
+ if (rc < 0)
+ {
+ close (fd);
+ throw libc_exception (errno,
+ string ("fstat ") + b_source0 + string (" ") + section);
+ }
+ r = MHD_create_response_from_fd (fs.st_size, fd);
+ }
+ else
+ r = MHD_create_response_from_fd (archive_entry_size(e), fd);
+
inc_metric ("http_responses_total","result",archive_extension + " archive");
- r = MHD_create_response_from_fd (archive_entry_size(e), fd);
if (r == 0)
{
if (verbose)
@@ -1812,7 +2009,9 @@ handle_buildid_r_match (bool internal_req_p,
add_mhd_response_header (r, "X-DEBUGINFOD-FILE", file.c_str());
add_mhd_last_modified (r, archive_entry_mtime(e));
if (verbose > 1)
- obatched(clog) << "serving archive " << b_source0 << " file " << b_source1 << endl;
+ obatched(clog) << "serving archive " << b_source0
+ << " file " << b_source1
+ << " section=" << section << endl;
/* libmicrohttpd will close it. */
if (result_fd)
*result_fd = fd;
@@ -1831,14 +2030,17 @@ handle_buildid_match (bool internal_req_p,
const string& b_stype,
const string& b_source0,
const string& b_source1,
+ const string& section,
int *result_fd)
{
try
{
if (b_stype == "F")
- return handle_buildid_f_match(internal_req_p, b_mtime, b_source0, result_fd);
+ return handle_buildid_f_match(internal_req_p, b_mtime, b_source0,
+ section, result_fd);
else if (b_stype == "R")
- return handle_buildid_r_match(internal_req_p, b_mtime, b_source0, b_source1, result_fd);
+ return handle_buildid_r_match(internal_req_p, b_mtime, b_source0,
+ b_source1, section, result_fd);
}
catch (const reportable_exception &e)
{
@@ -1913,6 +2115,7 @@ handle_buildid (MHD_Connection* conn,
if (artifacttype == "debuginfo") atype_code = "D";
else if (artifacttype == "executable") atype_code = "E";
else if (artifacttype == "source") atype_code = "S";
+ else if (artifacttype == "section") atype_code = "I";
else {
artifacttype = "invalid"; // PR28242 ensure http_resposes metrics don't propagate unclean user data
throw reportable_exception("invalid artifacttype");
@@ -1920,7 +2123,17 @@ handle_buildid (MHD_Connection* conn,
if (conn != 0)
inc_metric("http_requests_total", "type", artifacttype);
-
+
+ string section;
+ if (atype_code == "I")
+ {
+ if (suffix.size () < 2)
+ throw reportable_exception ("invalid section suffix");
+
+ // Remove leading '/'
+ section = suffix.substr(1);
+ }
+
if (atype_code == "S" && suffix == "")
throw reportable_exception("invalid source suffix");
@@ -1972,8 +2185,21 @@ handle_buildid (MHD_Connection* conn,
pp->bind(2, suffix);
pp->bind(3, canon_pathname(suffix));
}
+ else if (atype_code == "I")
+ {
+ pp = new sqlite_ps (thisdb, "mhd-query-i",
+ "select mtime, sourcetype, source0, source1, 1 as debug_p from " BUILDIDS "_query_d where buildid = ? "
+ "union all "
+ "select mtime, sourcetype, source0, source1, 0 as debug_p from " BUILDIDS "_query_e where buildid = ? "
+ "order by debug_p desc, mtime desc");
+ pp->reset();
+ pp->bind(1, buildid);
+ pp->bind(2, buildid);
+ }
unique_ptr<sqlite_ps> ps_closer(pp); // release pp if exception or return
+ bool do_upstream_section_query = true;
+
// consume all the rows
while (1)
{
@@ -1994,12 +2220,30 @@ handle_buildid (MHD_Connection* conn,
// Try accessing the located match.
// XXX: in case of multiple matches, attempt them in parallel?
auto r = handle_buildid_match (conn ? false : true,
- b_mtime, b_stype, b_source0, b_source1, result_fd);
+ b_mtime, b_stype, b_source0, b_source1,
+ section, result_fd);
if (r)
return r;
+
+ // If a debuginfo file matching BUILDID was found but didn't contain
+ // the desired section, then the section should not exist. Don't
+ // bother querying upstream servers.
+ if (!section.empty () && (sqlite3_column_int (*pp, 4) == 1))
+ {
+ struct stat st;
+
+ // For "F" sourcetype, check if the debuginfo exists. For "R"
+ // sourcetype, check if the debuginfo was interned into the fdcache.
+ if ((b_stype == "F" && (stat (b_source0.c_str (), &st) == 0))
+ || (b_stype == "R" && fdcache.probe (b_source0, b_source1)))
+ do_upstream_section_query = false;
+ }
}
pp->reset();
+ if (!do_upstream_section_query)
+ throw reportable_exception(MHD_HTTP_NOT_FOUND, "not found");
+
// We couldn't find it in the database. Last ditch effort
// is to defer to other debuginfo servers.
@@ -2074,6 +2318,11 @@ and will not query the upstream servers");
fd = debuginfod_find_source (client,
(const unsigned char*) buildid.c_str(),
0, suffix.c_str(), NULL);
+ else if (artifacttype == "section")
+ fd = debuginfod_find_section (client,
+ (const unsigned char*) buildid.c_str(),
+ 0, section.c_str(), NULL);
+
}
else
fd = -errno; /* Set by debuginfod_begin. */
diff --git a/debuginfod/debuginfod.h.in b/debuginfod/debuginfod.h.in
index 7d8e4972..69c9efd2 100644
--- a/debuginfod/debuginfod.h.in
+++ b/debuginfod/debuginfod.h.in
@@ -79,6 +79,12 @@ int debuginfod_find_source (debuginfod_client *client,
const char *filename,
char **path);
+int debuginfod_find_section (debuginfod_client *client,
+ const unsigned char *build_id,
+ int build_id_len,
+ const char *section,
+ char **path);
+
typedef int (*debuginfod_progressfn_t)(debuginfod_client *c, long a, long b);
void debuginfod_set_progressfn(debuginfod_client *c,
debuginfod_progressfn_t fn);
diff --git a/debuginfod/libdebuginfod.map b/debuginfod/libdebuginfod.map
index 93964167..6334373f 100644
--- a/debuginfod/libdebuginfod.map
+++ b/debuginfod/libdebuginfod.map
@@ -20,4 +20,5 @@ ELFUTILS_0.183 {
} ELFUTILS_0.179;
ELFUTILS_0.188 {
debuginfod_get_headers;
+ debuginfod_find_section;
} ELFUTILS_0.183;
diff --git a/doc/ChangeLog b/doc/ChangeLog
index 269ed06e..7b18dbef 100644
--- a/doc/ChangeLog
+++ b/doc/ChangeLog
@@ -1,3 +1,10 @@
+2022-10-31 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (notrans_dist_man3_MANS): Add debuginfod_find_section.3.
+ * debuginfod_find_section.3: New file.
+ * debuginfod_find_debuginfo.3: Document debuginfod_find_section.
+ * debuginfod.8: Document section webapi.
+
2022-10-28 Arsen Arsenović <arsen@aarsen.me>
* readelf.1: Document the --syms alias.
diff --git a/doc/Makefile.am b/doc/Makefile.am
index db2506fd..db5a842e 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -38,6 +38,7 @@ notrans_dist_man3_MANS += debuginfod_end.3
notrans_dist_man3_MANS += debuginfod_find_debuginfo.3
notrans_dist_man3_MANS += debuginfod_find_executable.3
notrans_dist_man3_MANS += debuginfod_find_source.3
+notrans_dist_man3_MANS += debuginfod_find_section.3
notrans_dist_man3_MANS += debuginfod_get_user_data.3
notrans_dist_man3_MANS += debuginfod_get_url.3
notrans_dist_man3_MANS += debuginfod_set_progressfn.3
diff --git a/doc/debuginfod.8 b/doc/debuginfod.8
index 7c1dc3dd..93db47e1 100644
--- a/doc/debuginfod.8
+++ b/doc/debuginfod.8
@@ -363,6 +363,17 @@ Note: the client should %-escape characters in /SOURCE/FILE that are
not shown as "unreserved" in section 2.3 of RFC3986. Some characters
that will be escaped include "+", "\\", "$", "!", the 'space' character,
and ";". RFC3986 includes a more comprehensive list of these characters.
+
+.SS /buildid/\fIBUILDID\fP/section\fI/SECTION\fP
+If the given buildid is known to the server, the server will attempt to
+extract the contents of an ELF/DWARF section named SECTION from the
+debuginfo file matching BUILDID. If the debuginfo file can't be found
+or the section has type SHT_NOBITS, then the server will attempt to extract
+the section from the executable matching BUILDID. If the section is
+succesfully extracted then this request results in a binary object
+of the section's contents. Note that this result is the raw binary
+contents of the section, not an ELF file.
+
.SS /metrics
This endpoint returns a Prometheus formatted text/plain dump of a
diff --git a/doc/debuginfod_find_debuginfo.3 b/doc/debuginfod_find_debuginfo.3
index 3dd83240..6469a3df 100644
--- a/doc/debuginfod_find_debuginfo.3
+++ b/doc/debuginfod_find_debuginfo.3
@@ -43,6 +43,12 @@ LOOKUP FUNCTIONS
.BI " int " build_id_len ","
.BI " const char *" filename ","
.BI " char ** " path ");"
+.BI "int debuginfod_find_section(debuginfod_client *" client ","
+.BI " const unsigned char *" build_id ","
+.BI " int " build_id_len ","
+.BI " const char * " section ","
+.BI " char ** " path ");"
+
OPTIONAL FUNCTIONS
@@ -98,6 +104,16 @@ accepts both forms. Specifically, debuginfod canonicalizes path names
according to RFC3986 section 5.2.4 (Remove Dot Segments), plus reducing
any \fB//\fP to \fB/\fP in the path.
+.BR debuginfod_find_section ()
+queries the debuginfod server URLs contained in
+.BR $DEBUGINFOD_URLS
+for the binary contents of an ELF/DWARF section contained in a debuginfo
+or executable file with the given \fIbuild_id\fP. \fIsection\fP should
+be the name of the desired ELF/DWARF section. If a server does not support
+section queries, debuginfod_find_section may query the server for the
+debuginfo and/or executable with \fIbuild_id\fP in order to retrieve
+and extract the section.
+
If \fIpath\fP is not NULL and the query is successful, \fIpath\fP is set
to the path of the file in the cache. The caller must \fBfree\fP() this value.
@@ -234,11 +250,8 @@ void *debuginfod_so = dlopen(DEBUGINFOD_SONAME, RTLD_LAZY);
.in
.SH "SECURITY"
-.BR debuginfod_find_debuginfo (),
-.BR debuginfod_find_executable (),
-and
-.BR debuginfod_find_source ()
-\fBdo not\fP include any particular security
+.BR debuginfod_find_* ()
+functions \fBdo not\fP include any particular security
features. They trust that the binaries returned by the debuginfod(s)
are accurate. Therefore, the list of servers should include only
trustworthy ones. If accessed across HTTP rather than HTTPS, the
diff --git a/doc/debuginfod_find_section.3 b/doc/debuginfod_find_section.3
new file mode 100644
index 00000000..16279936
--- /dev/null
+++ b/doc/debuginfod_find_section.3
@@ -0,0 +1 @@
+.so man3/debuginfod_find_debuginfo.3
diff --git a/tests/ChangeLog b/tests/ChangeLog
index a240a705..fb0dce82 100644
--- a/tests/ChangeLog
+++ b/tests/ChangeLog
@@ -1,3 +1,8 @@
+2022-10-31 Aaron Merey <amerey@redhat.com>
+
+ * Makefile.am (TESTS): Add run-debuginfod-section.sh.
+ * run-debuginfod-section.sh: New test.
+
2022-09-20 Yonggang Luo <luoyonggang@gmail.com>
* Makefile.am (EXTRA_DIST): Remove debuginfod-rpms/hello2.spec.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ced4a826..356b3fbf 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -247,7 +247,8 @@ TESTS += run-debuginfod-dlopen.sh \
run-debuginfod-x-forwarded-for.sh \
run-debuginfod-response-headers.sh \
run-debuginfod-extraction-passive.sh \
- run-debuginfod-webapi-concurrency.sh
+ run-debuginfod-webapi-concurrency.sh \
+ run-debuginfod-section.sh
endif
if !OLD_LIBMICROHTTPD
# Will crash on too old libmicrohttpd
@@ -554,6 +555,7 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \
run-debuginfod-response-headers.sh \
run-debuginfod-extraction-passive.sh \
run-debuginfod-webapi-concurrency.sh \
+ run-debuginfod-section.sh \
debuginfod-rpms/fedora30/hello2-1.0-2.src.rpm \
debuginfod-rpms/fedora30/hello2-1.0-2.x86_64.rpm \
debuginfod-rpms/fedora30/hello2-debuginfo-1.0-2.x86_64.rpm \
diff --git a/tests/run-debuginfod-section.sh b/tests/run-debuginfod-section.sh
new file mode 100755
index 00000000..a3cae349
--- /dev/null
+++ b/tests/run-debuginfod-section.sh
@@ -0,0 +1,135 @@
+#!/usr/bin/env bash
+#
+# Copyright (C) 2019-2021 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/debuginfod-subr.sh
+
+# for test case debugging, uncomment:
+set -x
+unset VALGRIND_CMD
+
+DB=${PWD}/.debuginfod_tmp.sqlite
+tempfiles $DB
+export DEBUGINFOD_CACHE_PATH=${PWD}/.client_cache
+
+# Set up directories for scanning
+mkdir F
+mkdir R
+cp -rvp ${abs_srcdir}/debuginfod-rpms R
+
+# This variable is essential and ensures no time-race for claiming ports occurs
+# set base to a unique multiple of 100 not used in any other 'run-debuginfod-*' test
+base=13000
+get_ports
+
+# We use -t0 and -g0 here to turn off time-based scanning & grooming.
+# For testing purposes, we just sic SIGUSR1 at the process.
+
+env LD_LIBRARY_PATH=$ldpath DEBUGINFOD_URLS= ${abs_builddir}/../debuginfod/debuginfod $VERBOSE \
+ -R R -F F -p $PORT1 -d $DB -t0 -g0 -v F > vlog$PORT1 2>&1 &
+PID1=$!
+tempfiles vlog$PORT1
+errfiles vlog$PORT1
+# Server must become ready
+wait_ready $PORT1 'ready' 1
+# And initial scan should be done
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 1
+
+export DEBUGINFOD_URLS=http://127.0.0.1:$PORT1
+
+# Check thread comm names
+ps -q $PID1 -e -L -o '%p %c %a' | grep groom
+ps -q $PID1 -e -L -o '%p %c %a' | grep scan
+ps -q $PID1 -e -L -o '%p %c %a' | grep traverse
+
+########################################################################
+
+# Compile a simple program, strip its debuginfo and save the build-id.
+# Also move the debuginfo into another directory so that elfutils
+# cannot find it without debuginfod.
+tempfiles prog.c
+echo "int main() { return 0; }" > ${PWD}/prog.c
+
+gcc -Wl,--build-id -g -o F/prog ${PWD}/prog.c
+
+testrun ${abs_top_builddir}/src/strip -g -f F/prog.debug ${PWD}/F/prog
+BUILDID=`env LD_LIBRARY_PATH=$ldpath ${abs_builddir}/../src/readelf \
+ -a F/prog | grep 'Build ID' | cut -d ' ' -f 7`
+
+kill -USR1 $PID1
+# Wait till both files are in the index.
+wait_ready $PORT1 'thread_work_total{role="traverse"}' 2
+wait_ready $PORT1 'thread_work_pending{role="scan"}' 0
+wait_ready $PORT1 'thread_busy{role="scan"}' 0
+
+########################################################################
+
+# Build-id for a file in the one of the testsuite's F31 rpms
+RPM_BUILDID=420e9e3308971f4b817cc5bf83928b41a6909d88
+
+# Download sections from files indexed with -F
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
+
+# Download sections from files indexed with -R
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
+
+# Verify that the downloaded files match the contents of the original sections
+tempfiles ${BUILDID}.debug_info
+objcopy F/prog.debug -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc $BUILDID.debug_info
+cmp ${BUILDID}.debug_info ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
+
+tempfiles ${BUILDID}.text
+objcopy F/prog -O binary --only-section=.text ${BUILDID}.text
+cmp ${BUILDID}.text ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
+
+# Download the original debuginfo/executable files.
+DEBUGFILE=`env LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find debuginfo $RPM_BUILDID`
+EXECFILE=`env LD_LIBRARY_PATH=$ldpath ${abs_top_builddir}/debuginfod/debuginfod-find executable $RPM_BUILDID`
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv debuginfo $BUILDID
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv executable $BUILDID
+
+if test "$(arch)" == "x86_64"; then
+ tempfiles DEBUGFILE.debug_info
+ objcopy $DEBUGFILE -O binary --only-section=.debug_info --set-section-flags .debug_info=alloc DEBUGFILE.debug_info
+ testrun diff -u DEBUGFILE.debug_info ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
+
+ tempfiles EXECFILE.text
+ objcopy $EXECFILE -O binary --only-section=.text EXECFILE.text
+ testrun diff -u EXECFILE.text ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
+fi
+
+# Kill the server.
+kill $PID1
+wait $PID1
+PID1=0
+
+# Delete the section files from the cache.
+rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.text
+rm -f ${DEBUGINFOD_CACHE_PATH}/${RPM_BUILDID}/section-.debug_info
+rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.text
+rm -f ${DEBUGINFOD_CACHE_PATH}/${BUILDID}/section-.debug_info
+
+# Verify that the client can extract sections from the debuginfo or executable
+# if they're already in the cache.
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $BUILDID .text
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .debug_info
+testrun ${abs_top_builddir}/debuginfod/debuginfod-find -vvv section $RPM_BUILDID .text
+
+exit 0
--
2.37.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] debuginfod: Support queries for ELF/DWARF sections
2022-11-01 4:53 ` Aaron Merey
@ 2022-11-01 20:29 ` Mark Wielaard
0 siblings, 0 replies; 4+ messages in thread
From: Mark Wielaard @ 2022-11-01 20:29 UTC (permalink / raw)
To: Aaron Merey; +Cc: elfutils-devel
Hi Aaron,
On Tue, Nov 01, 2022 at 12:53:41AM -0400, Aaron Merey wrote:
> Thanks again for the detailed review. I fixed the issues you pointed out.
This version looks really good. Please push, so it is included in the
release tomorrow.
> On Sat, Oct 29, 2022 at 8:29 PM Mark Wielaard <mark@klomp.org> wrote:
> > > @@ -1008,6 +1206,9 @@ debuginfod_query_server (debuginfod_client *c,
> > > snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> > > build_id_bytes, type, escaped_string);
> > > }
> > > + else if (section)
> > > + snprintf(data[i].url, PATH_MAX, "%s/%s/%s/%s", server_url,
> > > + build_id_bytes, type, section);
> >
> > Does the section part need to be path escaped? Like the filename is
> > with curl_easy_escape? And if it is how does that interact with the
> > cache file name?
>
> I assumed that section names would not contain '/'. However I noticed
> that if we call debuginfod_find_section with a section name containing
> '/', it will return -EINVAL. This triggers the downloading of debuginfo
> in order to attempt client-side extraction. This is a waste so I changed
> the client to path-escape the section name for caching purposes just like
> the source query filename.
It is highly unlikely a '/' is in the section name, but good to have
this covered anyway. Nicely extracted that escape function.
> > > + /* The servers may have lacked support for section queries. Attempt to
> > > + download the debuginfo or executable containing the section in order
> > > + to extract it. */
> >
> > This does somewhat defeat the purpose of just getting the section data
> > of course. So we could also just return EINVAL to the user here, so
> > they have to get the whole debuginfo themselves. But maybe just
> > pretending it worked is fine to keep the code path of the user
> > simpler?
>
> This keeps the code path simpler for the user and like Frank mentioned
> it enables an intermediate server to extract the section and provide
> it to the client. This server will also cache the debuginfo/executable,
> possibly speeding up future requests.
Ah, yes, the debuginfod server doesn use the client library too.
Thanks,
Mark
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-11-01 20:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-28 3:59 [PATCH v3] debuginfod: Support queries for ELF/DWARF sections Aaron Merey
2022-10-30 0:29 ` Mark Wielaard
2022-11-01 4:53 ` Aaron Merey
2022-11-01 20:29 ` Mark Wielaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).