Re: patch 2/2 debuginfod server etc.
On Tue, 2019-11-19 at 16:15 -0500, Frank Ch. Eigler wrote: > Hi - > > > > [...] What I want is simply make it easy for the user to say where > > they expect the sources are. So there is no surprises. > > If this were a mandate, it would be a hassle, for any build that's > more than one directory wide. It wouldn't be mandatory. It just wouldn't be the default. > > > The compiled-in default for the binary is off. The systemd service > > > default, it happens to be on, but it's configured to serve only > > > privileged directories that people with bad compilers cannot sneak > > > binaries into. People running personal servers can/should use -F as > > > they see fit. In the context of a normal workgroup - it's fine. > > > > So -F seems fine for the later, just not for the former. > > IMHO, even the former seems okay and even desirable: > > debuginfod -F /usr/lib/debug > > is a safe & easy way to relay the contents of all the debuginfo rpms > that were installed, to nearby clients. All those binaries come from > packages/distros, so are at least as high quality & trustworthiness as > the user's own. Again I offer to do an audit of some distro debuginfo > that all their source refs are milquetoast like /usr/include or > /usr/src/debug. Sure, you could use that if you wanted to share your whole build/source trees and don't mind serving any other files on some local network. I just think it shouldn't be the default. If you go look for odd paths in .debug files you probably will find them. We already know some builds generate and/or build files in /tmp or outside the src/builddir. I'll look to see what is necessary to make sure none of those leak out by default. > > > System certs do not serve to authenticate clients. Client > > > certificates are per-user things that come with their own management > > > headaches. Will think about authentication matters in the future. > > > > I thought ca-certificates.crt were normally used to authenticate > > remote servers. > > ca-certificates.crt types of files (or /usr/share/pki/ files) are the > trust roots for validating the *servers'* certificates. They are > generally provided by the distro, so can't possibly serve as unique > *client* authentication. I think we are talking past each other here. I am not really interested in "client certificates". I am simply interested in knowing what is done for outgoing https connections to be authenticated. What would it take to use the trust roots for validating the server certificates? Thanks, Mark
Re: patch 2/2 debuginfod server etc.
Hi - > I am simply interested in knowing what is done for outgoing https > connections to be authenticated. What would it take to use the trust > roots for validating the server certificates? OK, I misunderstood. Certificate-authority certificates in the trust root (compiled-in directories) serve to verify the signature chain on a certificate that a server sends to a client. libcurl already automatically does this verification, as does apprx. every other tls/ssl client in a distro. - FChE
Re: patch 1/2 debuginfod client
Hi,
On Tue, 2019-11-19 at 16:22 -0500, Frank Ch. Eigler wrote:
> On Tue, Nov 19, 2019 at 09:15:20PM +0100, Mark Wielaard wrote:
> > > That's what the doc says. What the code does, as far back as the year
> > > 2001, is have a static flag/counter in curl_global_init() that
> > > prevents multiple initialization.
> >
> > But the warning isn't about multiple initialization. It is about
> > initialization when other threads are running that might be using any
> > of the libcurl support libraries.
>
> Since 2001, the curl_global_init function does nothing to interfere
> with any libcurl activity, if the library is already initialized. Any
> call to the normal libcurl functions first calls this function. I
> guess I just fail to see a plausible problem scenario short of a
> minuscule race over incrementing an initialization counter, which is a
> race that every other libcurl user has accepted.
But it isn't just about interference with other libcurl activity. If
you look at the curl_global_init code you see that it actually calls a
lot of code in other libraries. It is the curl_global_init code that
shouldn't be run in a multi-threaded environment. That it is acceptable
to others doesn't immediately make it safe to use in our case. We are
slowly trying to make libdw.so into a multi-tread safe library and do
expect it to be used in multi-threaded code. We aren't fully there yet.
But it would be a shame to introduce more issues if we can prevent it.
> > > > That is why I think doing the dlopen of libdebuginfod.so eagerly from a
> > > > libdw.so constructor function or _init might be necessary to make sure
> > > > no other threads are running yet.
> > >
> > > That's not even enough for "sure" - if we're so deep in the
> > > hypotheticals hole, an application could be dlopen()ing libdw.so
> > > itself.
> >
> > It could, but I think that would be unlikely. We can at least document
> > that it is unsafe to use libdw.so with dlopen. But if it is done,
> > could we detect it and not do the loading of libdebuginfod.so in that
> > case?
>
> I don't know how to do that.
I assume you mean the second part. The attached is what I would propose
for the first part. Do you think that is a bad idea?
Thanks,
Mark
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 37a4c71f..ee604ad9 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -46,32 +46,7 @@ get_client (Dwfl *dwfl)
if (dwfl->debuginfod != NULL)
return dwfl->debuginfod;
- if (fp_debuginfod_begin == NULL)
-{
- void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
-
- if (debuginfod_so == NULL)
- debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
-
- if (debuginfod_so != NULL)
- {
- fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
- fp_debuginfod_find_executable = dlsym (debuginfod_so,
- "debuginfod_find_executable");
- fp_debuginfod_find_debuginfo = dlsym (debuginfod_so,
- "debuginfod_find_debuginfo");
- fp_debuginfod_end = dlsym (debuginfod_so, "debuginfod_end");
- }
-
- if (fp_debuginfod_begin == NULL
- || fp_debuginfod_find_executable == NULL
- || fp_debuginfod_find_debuginfo == NULL
- || fp_debuginfod_end == NULL)
- fp_debuginfod_begin = (void *) -1; /* never try again */
-}
-
- if (fp_debuginfod_begin != NULL
- && fp_debuginfod_begin != (void *) -1)
+ if (fp_debuginfod_begin != NULL)
{
dwfl->debuginfod = (*fp_debuginfod_begin) ();
return dwfl->debuginfod;
@@ -120,3 +95,37 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
if (c != NULL)
(*fp_debuginfod_end) (c);
}
+
+/* Try to get the libdebuginfod library functions to make sure
+ everything is initialized early. */
+void __attribute__ ((constructor))
+__libdwfl_debuginfod_init (void)
+{
+ void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
+
+ if (debuginfod_so == NULL)
+debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
+
+ if (debuginfod_so != NULL)
+{
+ fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
+ fp_debuginfod_find_executable = dlsym (debuginfod_so,
+ "debuginfod_find_executable");
+ fp_debuginfod_find_debuginfo = dlsym (debuginfod_so,
+ "debuginfod_find_debuginfo");
+ fp_debuginfod_end = dlsym (debuginfod_so, "debuginfod_end");
+
+ /* We either get them all, or we get none. */
+ if (fp_debuginfod_begin == NULL
+ || fp_debuginfod_find_executable == NULL
+ || fp_debuginfod_find_debuginfo == NULL
+ || fp_debuginfod_end == NULL)
+ {
+ fp_debuginfod_begin = NULL;
+ fp_debuginfod_find_executable = NULL;
+ fp_debuginfod_find_debuginfo = NULL;
+ fp_debuginfod_end = NULL;
+ dlclose (debuginfod_so);
+ }
+}
+}
[Bug libdw/25173] dwarf_getsrc_die fails for rust code
https://sourceware.org/bugzilla/show_bug.cgi?id=25173 --- Comment #7 from Milian Wolff --- hmm but in perfparser we have a workaround for missing aranges... I'll have to dig into this to see why it doesn't work here. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/25173] dwarf_getsrc_die fails for rust code
https://sourceware.org/bugzilla/show_bug.cgi?id=25173 --- Comment #8 from Mark Wielaard --- (In reply to Milian Wolff from comment #7) > hmm but in perfparser we have a workaround for missing aranges... I'll have > to dig into this to see why it doesn't work here. You also mention it is an issue with dwarf_getsrc_die in the subject. Which would indicate that it is some other issue, because when you call dwarf_getsrc_die you already would have found the DIE (by address). So maybe it is something else in your case? BTW. I do agree it would be nice if libdw handled missing debug_aranges. The problem is simply that you cannot know the difference between missing and not covered. Which means that for any address lookup that doesn't match an arange you would have to do some fallback lookup, which could be expensive. So still pondering how to handle this without penalizing debug files that do have correct and complete aranges (maybe we could just handle the case where there are just no .debug_aranges at all, and only then make some fallback code kick in?) -- You are receiving this mail because: You are on the CC list for the bug.
Re: patch 1/2 debuginfod client
Hi - > But it isn't just about interference with other libcurl activity. If > you look at the curl_global_init code you see that it actually calls > a lot of code in other libraries. It is the curl_global_init code > that shouldn't be run in a multi-threaded environment. That it is > acceptable to others doesn't immediately make it safe to use in our > case. [...] OK, I guess. Such concerns would be even better directed at the libraries that libcurl is using internally. > I assume you mean the second part. The attached is what I would propose > for the first part. Do you think that is a bad idea? It's mostly harmless, so if you like it, go for it. - FChE
