[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <CCV0JSXU6YMA.BT7V6OZT5HZE@mussels>
Date: Fri, 16 Jul 2021 22:10:57 -0300
From: Érico Nogueira <ericonr@...root.org>
To: <musl@...ts.openwall.com>
Cc: "Ariadne Conill" <ariadne@...eferenced.org>, "Timo Teras"
<timo.teras@....fi>
Subject: Re: option to enable eh_frame
Hullo,
Alpine MR is
<https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/22958>
On Fri Jul 16, 2021 at 10:08 PM -03, Ariadne Conill wrote:
> Hello,
>
> On Fri, 16 Jul 2021, Rich Felker wrote:
>
> > On Fri, Jul 16, 2021 at 05:06:48PM -0500, Ariadne Conill wrote:
> >> Hello,
> >>
> >> On Fri, 16 Jul 2021, Rich Felker wrote:
> >>
> >>> On Fri, Jul 16, 2021 at 12:16:25PM +0300, Timo Teras wrote:
> >>>> Hi,
> >>>>
> >>>> This has been discussed few times, and I know there are arguments also
> >>>> not to do this. But at this time we at Alpine think the reasons to keep
> >>>> eh_frame outweight the reasons to not include it.
> >>>
> >>> As explained on the tracker issue, what you'r requesting is a patch to
> >>> make a configure option that changes the public interface contract of
> >>> musl, making different interface contract profiles. This is not a
> >>> change to be made lightly. There has been *some* consideration in the
> >>> past for accepting this kind of option in the opposite direction:
> >>> omitting large functionality that might not be needed in some contexts
> >>> (like iconv), but where the default is to have it and omitting it is
> >>> just a choice particular users can make for working in a very
> >>> constrained environment. But if you add the option, you're essentially
> >>> making "having it" the de facto default, even if configure has
> >>> "disable" as the default. Once something is using it, there's an
> >>> implicit requirement to have it.
> >>>
> >>> Honestly, proposing that it always be available (or configurable but
> >>> on by default) would be less controversial than a configure option.
> >>> I'd still be against it but at least some of the badness is gone.
> >>>
> >>>> Main arguments against .eh_frame being:
> >>>>
> >>>> 1) Having .eh_frame makes it seem like C++ exception throwing works
> >>>> through C-library functions (e.g. throwing exception form qsort
> >>>> callback to return over qsort back to application).
> >>>>
> >>>> Counter arguments:
> >>>> - C++ exceptions is just one way to jump through musl functions.
> >>>> E.g. setjmp/longjmp can do that just fine even without .eh_frame
> >>>>
> >>>> 2) Having application unwind itself for backtrace printing purposes
> >>>> especially in signal handler is bad. This is agreed, but there's
> >>>> still other cases when unwinding is good for debugging, and other
> >>>> reasons. The fix for this root cause is to remove the unwinding from
> >>>> signal handlers.
> >>>
> >>> The debugger already can do debugging/unwinding because it has access
> >>> to the debug information (if you've installed it) and there is a
> >>> clear, understood-by-users contract that this information is not an
> >>> inherent part of the program but something optional for external
> >>> debugging tools only.
> >>>
> >>>> Arguments to have .eh_frame:
> >>>> - It allows debugging things even if musl-dbg is not or cannot be
> >>>> installed for some reason (e.g. gdb, valgrind), or is no longer
> >>>> available
> >>>> - libunwind/libexecinfo will start to work and give meaningful
> >>>> backtraces
> >>>
> >>> This is explicitly a reason not to. backtrace() considered harmful.
> >>>
> >>>> - Continuous kernel based profiling (e.g. perf record -g dwarf) will
> >>>> work
> >>>
> >>> This already works if you have debug info.
> >>>
> >>>> Given that the main arguments against are either making UB crash, or
> >>>> not the best fix, and keeping eh_frame enables useful features to work,
> >>>> I think it would make sense to allow enabling it.
> >>>>
> >>>> Please consider the the attached patch to make it a configure option to
> >>>> enable keeping eh_frame (defaulting still to not keeping it).
> >>>
> >>> You can solve this problem just as well for the things you want to
> >>> have work by including the (part of) the debug info you want in the
> >>> main libc.so binary: .debug_frame. Of course I can't stop Alpine from
> >>> doing it in a different way locally, but I would strongly recommend
> >>> you do that rather than making a contract that diverges from musl.
> >>
> >> The problem is that Alpine users want backtrace(3) to work. You
> >
> > This is a very different narrative from the request I received to take
> > this patch. I'm concerned about that.
> >
> >> consider it harmful, but users complain frequently. We also want
> >
> > Alpine is a security-oriented distribution. Doing introspective
> > debugging when a crash happens is *inviting* exploitation; the reason
> > that's the case has been discussed extensively before. Applications
> > trying to do this should be patched not to do it.
>
> Alpine does patch applications not to do it. This is not about enabling
> backtrace(3) for Alpine-shipped packages, but allowing end-users to use
> backtrace(3) and tools like Valgrind with their own applications that
> they
> build locally.
>
> While we advise users to install musl-dbg to solve these problems, they
> argue that basic functionality is available on their GNU/Linux
> distribution without having to install libc6-dbg or glibc-debuginfo or
> whatever equivalent package.
It would probably double the size, but could alpine offer a
"development" image that already comes with musl-dbg (assuming the
dislike for installing musl-dbg is about having to add it to Dockerfiles
or w/e)?
Void avoids this "debate" by simply not stripping libc (either musl or
glibc), since it makes local debugging with gdb/valgrind so much easier.
I wasn't aware libexecinfo also took so much advantage of it.
>
> This comes up a lot with users using Alpine as a mechanism to get a
> musl-based runtime environment for Docker. The lifecycle for
> applications
> running inside Docker is not particularly friendly to traditional debug
> methods, and so having working backtrace(3) is helpful for that
> scenario,
> despite unwinding in a SIGSEGV handler being dangerous.
backtrace(3) in dev can easily become backtrace(3) in prod; I'd
certainly be against encouraging proliferation of its usage. However, I
haven't had to deal with corporate distro clients either, so I don't
have a handle on the complete picture.
Maybe document some setup to export core dumps to outside of a container
in the case of crashes? (is that doable? I'm not a container person)
Catching SIGSEGV at all has always seemed like a pretty weird idea to
me.
>
> >> C++ exception throwing across libc boundary to work, but admittedly
> >> that is a lot harder to achieve.
> >
> > Do you have in mind some place where this would be remotely valid
> > usage? I don't mean per the spec, where it's never valid, but some
> > kind of "at least this makes sense someone could want to do it" sense.
> >
> > There are only a few places where the application can even see an
> > environment called back from libc. Most of these are things like
> > comparison functions that are required to be pure. There is no way in
> > which throwing from them could ever make sense, because throwing is
> > not a pure operation. Another is things like enumeration callbacks.
> > Most of these necessarily have inconsistent state and/or resources
> > held that would be lost if you could asynchronously jump out of them,
> > and most of them have a way for the callback to request the operation
> > to stop. This means that if you want to use exceptions with them, the
> > top-level callback just has to catch the exception and convert it to a
> > return value. The same applies for things like fopencookie.
>
> This is just a commonly requested thing. Alpine does not really care
> about enabling this usecase right now.
I too would be interested to know what examples came with such requests,
if you can share them.
>
> I should clarify what Alpine wants here: we want users who are building
> their applications on Alpine to not be surprised by musl's behaviour.
> That can be solved either through allowing exceptions to cross libc
> (like
> on GNU/Linux with glibc) or through documentation. I prefer to solve
> this
> issue with documentation, personally.
Do you think this sort of thing fits better in the wiki [1] or in the
WIP musl manual [2]?
Part of the reason people choose musl at all is because of its design
choices, which make it more than just a glibc clone. I'm not sure it's
possible to have it all - benefits of musl *and* glibc in the same
package. Hopefully that notion can appease those who specifically chose
musl, but it would probably be discouraging to anyone who wants this
functionality to debug their CI failures, where musl is more about
coverage and less about a platform choice.
Links for anyone unaware, though they are available from the homepage:
[1] https://wiki.musl-libc.org/
[2] http://musl.libc.org/manual.html
>
> But regardless, backtrace(3) should work for our users if they wish to
> use
> it in their downstream applications.
>
> >> I am concerned about the unilateral approach we have taken to enable
> >> backtrace(3) though, if we are forking the musl ABI, we probably
> >> will wind up forking the musl API too to add user-requested
> >> functionality.
> >
> > This would be very unfortunate, and would make me actively recommend
> > against use of Alpine. One of the core values of musl is consensus
> > process, not attempts to unilaterally force something like this,
> > especially when it's been done in a way that feels misleading.
> > Communication with me/upstream musl started out as presenting this as
> > being about debugging, but it feels like that was a bait-and-switch
> > for getting something else.
>
> It is about debugging, but specifically enabling downstream users to do
> basic debugging without having to install the musl-dbg package. This is
> commonly requested both in public Alpine channels, and also in various
> support engagements Alpine developers have with their customers in a
> personal capacity. It has been a point of contention for many years in
> the Alpine community, as a result.
>
> Nonetheless, I am disappointed that the implementation of `.eh_frame`
> was
> done unilaterally. I should have responded sooner on the tracker item,
> but had forgotten about it until it was implemented today.
>
> My concern is that Alpine is entering a "we broke it, we bought it"
> situation with this kind of unilateral decision making. While we have
> built a very nice platform around musl, I don't want to be in a
> situation
> where we "own" a substantively modified version of musl. We have been in
> that situation before with uClibc, and we do not want to go back there.
>
> Accordingly, I would strongly prefer that Alpine try to solve this issue
> with `.debug_frame` first, and then we can look at `.eh_frame` later if
> it
> winds up being insufficient for making basic debug functionality work
> (on
> at least the same level as glibc).
>
> >> We (Alpine) should consider whether we actually want this.
> >> Historically, we have been able to find compromises that allow us to
> >> enable the user requests in a way that Rich finds acceptable.
> >
> > Yes. As a distro, you oftentimes have users asking for something
> > that's wrong, that shouldn't exist. One core job of a maintainer -- a
> > distro maintainer too -- is saying no. Here, users seem to be asking
> > for something that makes software less secure, and that's not a
> > portable feature one can rely on having.
>
> I am not convinced that backtrace(3) and basic Valgrind functionality
> makes Alpine less secure. Making the software *we* ship to downstream
> users spew backtrace logs all over the place when it crashes or hits an
> assertion would make Alpine less secure, but we don't and won't be doing
> that.
Can you share an example of valgrind output before and after the
.eh_frame change? It would be nice to have some more context.
>
> Ariadne
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
