[Bug debuginfod/25448] Extend debuginfod metrics
https://sourceware.org/bugzilla/show_bug.cgi?id=25448 --- Comment #2 from Martin Liška --- I like the suggested patch! -- You are receiving this mail because: You are on the CC list for the bug.
Re: patch PR25583: debuginfod bsdtar for deb
Hi - > Like this? > (/usr/bin/dkpg-deb is the usual installation path) pushed to master after testing on f30 with and without dpkg. - FChE
Re: patch PR25583: debuginfod bsdtar for deb
Hi -
Like this?
(/usr/bin/dkpg-deb is the usual installation path)
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 5e19db517472..58ba85cf3664 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-26 Frank Ch. Eigler
+
+ * debuginfod.cxx (parse_opt): For -U, prefer dpkg-deb
+ after all if access(3)-able, fallback to bsdtar.
+
2020-03-25 Frank Ch. Eigler
* debuginfod.cxx (parse_opt): Associate a bsdtar subshell with
diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx
index c03bbf922f59..9e8d55609c56 100644
--- a/debuginfod/debuginfod.cxx
+++ b/debuginfod/debuginfod.cxx
@@ -429,8 +429,16 @@ parse_opt (int key, char *arg,
scan_archives[".rpm"]="cat"; // libarchive groks rpm natively
break;
case 'U':
- scan_archives[".deb"]="(bsdtar -O -x -f - data.tar.xz)<";
- scan_archives[".ddeb"]="(bsdtar -O -x -f - data.tar.xz)<";
+ if (access("/usr/bin/dpkg-deb", X_OK) == 0)
+{
+ scan_archives[".deb"]="dpkg-deb --fsys-tarfile";
+ scan_archives[".ddeb"]="dpkg-deb --fsys-tarfile";
+}
+ else
+{
+ scan_archives[".deb"]="(bsdtar -O -x -f - data.tar.xz)<";
+ scan_archives[".ddeb"]="(bsdtar -O -x -f - data.tar.xz)<";
+}
// .udeb too?
break;
case 'Z':
Re: PR25369 slice 3/3: debuginfod header relay
Hi Frank,
On Thu, 2020-03-26 at 18:54 -0400, Frank Ch. Eigler wrote:
> > Normally a user client doesn't really need to know much about whether
> > or how the file information is fetched. But in this case we have a http
> > transport specific thing (and we just added support for file:// URLs).
> > It also seems very curl specific. To use it correctly you need to know
> > the structure of HTTP request header/value and how CURLOPT_HTTPHEADER
> > uses the given string to add, replace, delete (end with a colon) or
> > create a header with no value (adding a semicolon at the end).
>
> I don't see why this would be true. The function is named "add", and
> the documentation says "Header: value". This does not expose CURL
> replace/delete operations.
>
> > I think that we should be very explicit that this API is an optional
> > hint only. That the user must not rely on the header actually being
> > used or forwarded through the transport. And that it is only to be used
> > for optional logging.
>
> I guess I don't see a problem here. HTTP request headers are by
> definition optional things. How it's used --- why would we want to
> dictate "only ... for" anything? We do what we do.
But what we do is technically just pass the given string to curl as is.
It is just that I know what people will do is read the source code and
read the CURLOPT_HTTPHEADER documentation and think they can just use
it like that. I just want it to be super clear that we reserve the
right to break someones code if they rely on the current
implementation.
> > I am especially worried that people will start using the API to
> > override and/or disable internal (curl) headers, which would mean we
> > are stuck with all kinds of curl specifics. So ideally I would like to
> > programmatically prevent that. Or at document very clearly that if
> > someone does that, that is totally unsupported and we are free to break
> > such usage in any (minor) update/bug fix release.
>
> People who manage to muck up curl internal headers would already be
> violating our documentation ("add", "Header: value"). I just don't
> see any threat here, let alone any sort of special technical
> obligation on us to stop this or offer scare stories. The function
> simply does what it says on the tin.
Except that people will figure out what it really does. I don't think
it is a scare story to explicitly say: "Note that the current
implementation uses libcurl, but you shouldn't rely on that fact. The
only supported usage of this method is for adding an optional header
which might or might not be passed through to the server."
> > > void
> > > debuginfod_end (debuginfod_client *client)
> > > {
> > > + if (client) /* make it safe to be invoked with NULL */
> > > +curl_slist_free_all (client->headers);
> > >free (client->url);
> > >free (client);
> > > }
> >
> > It seems a good idea to allow debuginfod_end (NULL). But if client
> > would be NULL then the free (client->url) would also crash.
> > So just put everything under the if (client) { ... }
>
> Or if (!client) return or whatever.
Yes, as long as the behavior is consistent.
> > > {
> > > + add_default_headers(client);
> > >return debuginfod_query_server(client, build_id, build_id_len,
> > > "source", filename, path);
> > > }
> >
> > Why not have add_default_headers at the top of the
> > debuginfod_query_server () function instead of repeating it 3 times
> > here?
>
> Sure, OK.
>
>
> > So if there is any way to check whether this is overriding and/or
> > deleting an existing internal curl header here it would be really good
> > to add that here and simply reject that.
>
> I guess I don't see why we would voluntarily undertake the
> responsibility for detailed classification of already
> documentation-violating input. Remember, this is just a client. If
> some user really needs this sort of goofy capability, they could have
> always just written their code to libcurl directly. We can't stop
> them.
Sure. It is simple to write your own client code using libcurl if you
want to. And it might be too hard to sanity check the input. If it is,
too bad. But if it is easy to check then I think we should simply do
that to catch user mistakes.
> > >struct MHD_Response *r = 0;
> > >try
> > > {
> > > - r = handle_buildid (buildid, "debuginfo", "", &alt_fd);
> > > + r = handle_buildid (0, buildid, "debuginfo", "", &alt_fd);
> > > }
> >
> > 0 instead of NULL?
> > I believe we went over this before, 0 is more C++?
>
> Yes.
OK. I guess I got stuck in C language mindset.
> > > +/* Add an outgoing HTTP request "Header: Value". Copies string. */
> > > +int debuginfod_add_http_header (debuginfod_client *client, const char*
> > > header);
> >
> > So here at least add "optional header" or something. A warning that
> > this is a hint only and might be ignored depending on backend
Re: PR25369 slice 3/3: debuginfod header relay
Hi - > > I guess I don't see a problem here. HTTP request headers are by > > definition optional things. How it's used --- why would we want to > > dictate "only ... for" anything? We do what we do. > > But what we do is technically just pass the given string to curl as is. > It is just that I know what people will do is read the source code and > read the CURLOPT_HTTPHEADER documentation and think they can just use > it like that. [...] > Except that people will figure out what it really does. But documentation would do nothing to dissuade such people! If we have to change away from libcurl, they'll just read the new code too. If they need these funky header manipulations anyway, they'll just change OUR code. > I don't think it is a scare story to explicitly say: "Note that the > current implementation uses libcurl, but you shouldn't rely on that > fact. The only supported usage of this method is for adding an > optional header which might or might not be passed through to the > server." OK, please feel free to add any such text that makes you feel more comfortable. > Sure. It is simple to write your own client code using libcurl if you > want to. And it might be too hard to sanity check the input. If it is, > too bad. But if it is easy to check then I think we should simply do > that to catch user mistakes. We were talking about people who read the code to work around the documented pattern. These would not be user mistakes. We're proposing protecting someone not from their mistakes, but their deliberate reverse-engineering. - FChE
Re: patch PR25722: /path/name based debuginfod-find & API lookups
Hi Frank, On Wed, 2020-03-25 at 21:39 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > commit b27d38f7eed6d99715fd1cc8a70b0a6a2b04f0ce (HEAD -> fche/pr25722) > Author: Frank Ch. Eigler > Date: Wed Mar 25 21:36:51 2020 -0400 > > PR25722: debuginfod client api: accept /path/names in place of buildid hex > > As a convenience, this extends the debuginfod find functions (and thus > debuginfod-find) to accept a /path/name to an ELF binary as an > alternative to a hexadecimal string for buildid. Doc & testing incl. I like the functionality for debuginfod-find, but think it is not really appropriate for the debuginfod client API. It is a little hacky. The user code has to know that it has to call elf_version () first. The interface using path names is somewhat awkward since users probably already have an file descriptor or ELF handle open for the file. It adds a dependency on libelf and libdw for two convenience function calls that the user could do themselves. And it only handles the main build-id in the file, e.g. you cannot easily use it for fetching the multi/alt-file. I think we should move the functionality as is just into debuginfo-find and experiment a bit with it before adding it as a public API to the debuginfod-client API. In debuginfod-find the code would also be slightly simpler since it doesn't have to convert to a hex-string first, it can just pass the build-id bytes as is. Cheers, Mark
Re: patch PR25448: more debuginfod prometheus metrics
Hi Frank,
On Thu, 2020-03-26 at 16:46 -0400, Frank Ch. Eigler via Elfutils-devel wrote:
> commit 34d851344dbaa5fd00b4368742839f86203202a1 (HEAD ->
> fche/pr25448)
> Author: Frank Ch. Eigler
> Date: Thu Mar 26 16:44:20 2020 -0400
>
> PR25448: debuginfod: add transfer performance metrics
>
> We now export metrics related to the time taken and data sent,
> from which prometheus type tools can compute nice time series
> with averages.
>
> http_responses_duration_milliseconds_count{code="200"} 63
> http_responses_duration_milliseconds_count{code="404"} 2
> http_responses_duration_milliseconds_count{code="503"} 1
> http_responses_duration_milliseconds_sum{code="200"} 66
> http_responses_duration_milliseconds_sum{code="404"} 2
> http_responses_duration_milliseconds_sum{code="503"} 0
> http_responses_transfer_bytes_count{code="200"} 63
> http_responses_transfer_bytes_count{code="404"} 2
> http_responses_transfer_bytes_count{code="503"} 1
> http_responses_transfer_bytes_sum{code="200"} 425177
> http_responses_transfer_bytes_sum{code="404"} 18
> http_responses_transfer_bytes_sum{code="503"} 37
>
> Signed-off-by: Frank Ch. Eigler
>
> diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
> index 5e19db517472..ebb67b0df378 100644
> --- a/debuginfod/ChangeLog
> +++ b/debuginfod/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-03-26 Frank Ch. Eigler
> +
> + * debuginfod.cxx (handler_cb): Export two families of metrics for
> + prometheus traffic analysis: response times and data amounts.
This looks nice. Do we have to document these anywhere, or are
prometheus metrics "self documenting"? I assume tools just take what
they can get from the json export and create graphs based on the name
count/sum patterns?
Thanks,
Mark
[PATCH] debuginfod: Disallow --port=0.
When starting debuginfod with --port=0 it would start using a random port (possibly different for ipv4 and ipv6). This seems to be an accidental and not very useful functionality. Just produce an error when started with --port=0. https://sourceware.org/bugzilla/show_bug.cgi?id=25728 Signed-off-by: Mark Wielaard --- debuginfod/ChangeLog | 4 debuginfod/debuginfod.cxx | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog index 58ba85cf..018d62f6 100644 --- a/debuginfod/ChangeLog +++ b/debuginfod/ChangeLog @@ -1,3 +1,7 @@ +2020-03-27 Mark Wielaard + + * debuginfod.cxx (parse_opt): Check port is not zero. + 2020-03-26 Frank Ch. Eigler * debuginfod.cxx (parse_opt): For -U, prefer dpkg-deb diff --git a/debuginfod/debuginfod.cxx b/debuginfod/debuginfod.cxx index 9e8d5560..84eeb78d 100644 --- a/debuginfod/debuginfod.cxx +++ b/debuginfod/debuginfod.cxx @@ -422,7 +422,8 @@ parse_opt (int key, char *arg, case 'v': verbose ++; break; case 'd': db_path = string(arg); break; case 'p': http_port = (unsigned) atoi(arg); - if (http_port > 65535) argp_failure(state, 1, EINVAL, "port number"); + if (http_port == 0 || http_port > 65535) +argp_failure(state, 1, EINVAL, "port number"); break; case 'F': scan_files = true; break; case 'R': -- 2.18.2
[Bug debuginfod/25728] Starting debubinfod with --port=0 is not documented
https://sourceware.org/bugzilla/show_bug.cgi?id=25728 Mark Wielaard changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #2 from Mark Wielaard --- Proposed to just disable this undocumented "feature" because it doesn't seem very useful: https://sourceware.org/pipermail/elfutils-devel/2020q1/002568.html -- You are receiving this mail because: You are on the CC list for the bug.
Re: patch PR25448: more debuginfod prometheus metrics
Hi - On Fri, Mar 27, 2020 at 03:47:27PM +0100, Mark Wielaard wrote: > This looks nice. Do we have to document these anywhere, or are > prometheus metrics "self documenting"? I assume tools just take what > they can get from the json export and create graphs based on the name > count/sum patterns? Yeah, the naming convention of related metrics (_sum, _count) allows averages to be auto-calculated. No, we don't document any of these formally (and note that in the man page). Thanks for the review. - FChE
[Bug debuginfod/25448] Extend debuginfod metrics
https://sourceware.org/bugzilla/show_bug.cgi?id=25448 Frank Ch. Eigler changed: What|Removed |Added Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED --- Comment #3 from Frank Ch. Eigler --- pushed -- You are receiving this mail because: You are on the CC list for the bug.
Re: elfutils 0.179 release end of the week?
Hi Frank, On Tue, 2020-03-24 at 08:40 -0400, Frank Ch. Eigler via Elfutils-devel wrote: > Are there any other "urgent" bug fixes pending? > > Yeah, a few more debuginfod bits please. I'll work all out today and > tomorrow on as many as possible of PR25548, PR25367, PR25366, > PR25448, PR25583. Thanks so much for working so hard the last few days. I am afraid I wasn't as productive reviewing. But I believe we are mostly there: PR25548 also support canonicalized source-file name lookups in webapi commit d63a809da467e646480c273b8eb276401679d2bb PR25367 web request status logging improvements commit 439641e52a3258ddfa1f5de8bbcdb1530fc1 PR25366 debuginfod scan: more progress/status Nothing yet, but is it very urgent? PR25448 Extend debuginfod metrics commit 0b9eb740eb8cd86ce3bffd0278135eba98c2e8a2 PR25583 Use libarchive to extract .deb packages commit bde8d7ec3fb1780f8178586533d76a959bde89ab commit 426a000dc59bd824de86d20b1e4d772340031067 There are also: PR25722: /path/name based debuginfod-find & API lookups Which is fine for debuginfod-find, but I find the new API awkward and would prefer not to add that part. PR25369 slice 3/3: debuginfod header relay Which seems OK code wise, but we are bickering about how to document it (and whether we can double check the input arguments). Anything else? It would be nice to get those last two issues in before the release. Let them ripe during the weekend to give the buildbots or other testers some time to digest and do a release on Monday or Tuesday? Cheers, Mark
Re: elfutils 0.179 release end of the week?
Hi - > There are also: > > PR25722: /path/name based debuginfod-find & API lookups > Which is fine for debuginfod-find, but I find the new API awkward and > would prefer not to add that part. Understood, will post a version with that. > PR25369 slice 3/3: debuginfod header relay > Which seems OK code wise, but we are bickering about how to document it > (and whether we can double check the input arguments). Will push this one and let you add docs if you like. > It would be nice to get those last two issues in before the release. > Let them ripe during the weekend to give the buildbots or other testers > some time to digest and do a release on Monday or Tuesday? Sounds like a plan. I might make a start at the elfutils-bootstrap packaging but may not get it done. It would barely affect the code base proper. - FChE
[Bug debuginfod/25739] New: client cache sometimes has incorrect mtimes for downloaded files
https://sourceware.org/bugzilla/show_bug.cgi?id=25739 Bug ID: 25739 Summary: client cache sometimes has incorrect mtimes for downloaded files Product: elfutils Version: unspecified Status: UNCONFIRMED Severity: normal Priority: P2 Component: debuginfod Assignee: unassigned at sourceware dot org Reporter: eschwartz at archlinux dot org CC: elfutils-devel at sourceware dot org Target Milestone: --- When running debuginfod-find repeatedly against some files, it seems that if you rm a file from the cache, it spoils the mtime of all future files being downloaded (so that it results in the current time instead). As a result my gdb-git complains "warning: Source file is more recent than executable." After some IRC debugging, fche suggested trying to run the server using --fdcache-fds=0 which solved the issue. 12:54 PM <@fche> we just neglect to save the timestamps of fdcache'd files extracted from archives 12:54 PM <@fche> ok 12:54 PM <@fche> betcha if you run debuginfod with--fdcache-fds=0 12:54 PM <@fche> and even --fdcache-prefetch=0 12:54 PM <@fche> things will work -- You are receiving this mail because: You are on the CC list for the bug.
Re: patch PR25722: /path/name based debuginfod-find & API lookups
Hi -
Like this?
Author: Frank Ch. Eigler
Date: Fri Mar 27 20:47:45 2020 -0400
PR25722: debuginfod-find: accept /path/names in place of buildid hex
Extend the debuginfod-find command line interface to permit
/file/names instead of only buildid hexadecimal strings.
v2: move code into debuginfod-find; client API remains buildid only.
Signed-off-by: Frank Ch. Eigler
diff --git a/debuginfod/ChangeLog b/debuginfod/ChangeLog
index 3329be3510d2..6fc4c0981687 100644
--- a/debuginfod/ChangeLog
+++ b/debuginfod/ChangeLog
@@ -1,3 +1,9 @@
+2020-03-27 Frank Ch. Eigler
+
+ * debuginfod-find.c (main): Extract buildid from /binary/ if
+ given instead of hex string.
+ * Makefile.am: Add elfutils library prereqs for debuginfod-find.
+
2020-03-24 Frank Ch. Eigler
* debuginfod.h, libdebuginfod.map: New functions for _add_url_header.
diff --git a/debuginfod/Makefile.am b/debuginfod/Makefile.am
index 52ead30aebf8..51965f65dbb7 100644
--- a/debuginfod/Makefile.am
+++ b/debuginfod/Makefile.am
@@ -62,7 +62,7 @@ debuginfod_SOURCES = debuginfod.cxx
debuginfod_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod)
$(libmicrohttpd_LIBS) $(libcurl_LIBS) $(sqlite3_LIBS) $(libarchive_LIBS)
-lpthread -ldl
debuginfod_find_SOURCES = debuginfod-find.c
-debuginfod_find_LDADD = $(libeu) $(libdebuginfod)
+debuginfod_find_LDADD = $(libdw) $(libelf) $(libeu) $(libdebuginfod)
noinst_LIBRARIES = libdebuginfod.a
noinst_LIBRARIES += libdebuginfod_pic.a
diff --git a/debuginfod/debuginfod-find.c b/debuginfod/debuginfod-find.c
index 400c268224fb..83a43ce4be2e 100644
--- a/debuginfod/debuginfod-find.c
+++ b/debuginfod/debuginfod-find.c
@@ -25,6 +25,10 @@
#include
#include
#include
+#include
+#include
+#include
+#include
/* Name and version of program. */
@@ -39,8 +43,12 @@ static const char doc[] = N_("Request debuginfo-related
content "
/* Strings for arguments in help texts. */
static const char args_doc[] = N_("debuginfo BUILDID\n"
+ "debuginfo PATH\n"
"executable BUILDID\n"
- "source BUILDID /FILENAME");
+ "executable PATH\n"
+ "source BUILDID /FILENAME\n"
+ "source PATH /FILENAME\n");
+
/* Definitions of arguments for argp functions. */
static const struct argp_option options[] =
@@ -86,6 +94,8 @@ static struct argp argp =
int
main(int argc, char** argv)
{
+ elf_version (EV_CURRENT);
+
client = debuginfod_begin ();
if (client == NULL)
{
@@ -105,19 +115,61 @@ main(int argc, char** argv)
return 1;
}
- int rc;
+ /* If we were passed an ELF file name in the BUILDID slot, look in there. */
+ unsigned char* build_id = (unsigned char*) argv[remaining+1];
+ int build_id_len = 0; /* assume text */
+
+ int any_non_hex = 0;
+ int i;
+ for (i = 0; build_id[i] != '\0'; i++)
+if ((build_id[i] >= '0' && build_id[i] <= '9') ||
+(build_id[i] >= 'a' && build_id[i] <= 'f'))
+ ;
+else
+ any_non_hex = 1;
+
+ int fd = -1;
+ Elf* elf = NULL;
+ if (any_non_hex) /* raw build-id */
+{
+ fd = open ((char*) build_id, O_RDONLY);
+ if (fd < 0)
+fprintf (stderr, "Cannot open %s: %s\n", build_id, strerror(errno));
+}
+ if (fd >= 0)
+{
+ elf = elf_begin (fd, ELF_C_READ_MMAP_PRIVATE, NULL);
+ if (elf == NULL)
+fprintf (stderr, "Cannot elf_begin %s: %s\n", build_id,
elf_errmsg(-1));
+}
+ if (elf != NULL)
+{
+ const void *extracted_build_id;
+ ssize_t s = dwelf_elf_gnu_build_id(elf, &extracted_build_id);
+ if (s > 0)
+{
+ /* Success: replace the build_id pointer/len with the binary blob
+ that elfutils is keeping for us. It'll remain valid until
elf_end(). */
+ build_id = (unsigned char*) extracted_build_id;
+ build_id_len = s;
+}
+ else
+fprintf (stderr, "Cannot extract build-id from %s: %s\n", build_id,
elf_errmsg(-1));
+}
+
char *cache_name;
+ int rc = 0;
/* Check whether FILETYPE is valid and call the appropriate
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,
- (unsigned char *)argv[remaining+1], 0,
+ build_id, build_id_len,
&cache_name);
else if (strcmp(argv[remaining], "executable") == 0)
rc = debuginfod_find_executable(client,
- (unsigned char *)argv[remaining+1], 0,
+build_id, build_id_len,
&cache_name);
else if (strcmp(argv[remaining], "source") == 0)
{
@@ -126,8
[Bug tools/23787] eu-size: Bad handling of ar files inside are files
https://sourceware.org/bugzilla/show_bug.cgi?id=23787 viren changed: What|Removed |Added CC||rajputveer8055 at gmail dot com --- Comment #15 from viren --- For reference this was assigned CVE-2018-18520. Note that the description of the CVE is misleading. The bug is in eu-size, not in libelf elf_end. https://gadgetsmagnet.com https://technorazor.com -- You are receiving this mail because: You are on the CC list for the bug.
[Bug tools/23787] eu-size: Bad handling of ar files inside are files
https://sourceware.org/bugzilla/show_bug.cgi?id=23787 --- Comment #16 from viren --- POC2 Please use " ./eu-size $POC " to reproduce this bug. This bug was discovered by NTU Cyber-Security-Lab, for fuzzing research work. If you have any questions, please let me know. https://technorazor.com/best-anime-website/ https://technorazor.com/free-sports-streaming-sites/ https://technorazor.com/youtube-multi-downloader/ https://technorazor.com/9xmovies/ https://technorazor.com/fmovies-watch-movies-online/ https://technorazor.com/watch-cartoons-online/ https://technorazor.com/gogoanime/ https://technorazor.com/putlocker-alternative/ https://technorazor.com/tamilrockers/ https://technorazor.com/9xmovies/ https://technorazor.com/fmovies-watch-movies-online/ https://technorazor.com/animefreak/ https://technorazor.com/123movies-online/ https://technorazor.com/kissanime-alternatives/ https://technorazor.com/9anime-alternatives/ https://technorazor.com/couchtuner-alternative/ https://technorazor.com/movierulz/ https://technorazor.com/mp4mania/ https://technorazor.com/jalshamoviez-hd-movies/ https://technorazor.com/cinemavilla-2019-download-tamil-telugu-kannadamovies-online-free/ -- You are receiving this mail because: You are on the CC list for the bug.
