From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ua1-x931.google.com (mail-ua1-x931.google.com [IPv6:2607:f8b0:4864:20::931]) by sourceware.org (Postfix) with ESMTPS id 98047385840D for ; Fri, 19 Nov 2021 19:56:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 98047385840D Received: by mail-ua1-x931.google.com with SMTP id o1so23541525uap.4 for ; Fri, 19 Nov 2021 11:56:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=ThnqpNKDbHlPvumeIj7HGxudDvcsTCD7Qs3AMyaqQ6U=; b=aupx6DaNL0+esv8noUdHEBMksG4CMmu+l6P5DnwLoh/LAKCR1MrszCsYtmRNTe/42b 55mXFqbVE42ZcXGUqp5gi04R3fTm0MQX0asYUdMuKqS/DucEmMMTlAxNIoUU+bjUUweI wsw2SbqbBFLAiAaLh4Ym0H8nqI6exnku20vX1IfE/BneXcdp8bl/8gT7IJy49AoXqi81 ocX9a90VcxS/9Kx0sUAFAboBTsBsPocpjjvx14XkCtWXkF83luywXEoh8jhPZSDUrEpx TOWglFTTeUJ/2eAMv30Doiej0kS7JYUmdp9asKzzwLhpzPUsaHjQSoODtEZtqj8SGlF5 5hdA== X-Gm-Message-State: AOAM532jz51sHPoaLbD5WbJtDHbNYSVNwWUlKhTfxX10Wp2c/8+thglL FVIE/eJZ/WdeOGrCiVm2T/LEUUbLTSPhbA== X-Google-Smtp-Source: ABdhPJz1BHfF50ndzV778RuWopPyAo3OY/xI1zoq03eq3ex8iX5Vk/49s09mWBf1L6JGkJaqZNZRkw== X-Received: by 2002:a67:de88:: with SMTP id r8mr94741245vsk.15.1637351789061; Fri, 19 Nov 2021 11:56:29 -0800 (PST) Received: from ?IPV6:2804:431:c7ca:66dc:bc2c:d33d:22b3:25d7? ([2804:431:c7ca:66dc:bc2c:d33d:22b3:25d7]) by smtp.gmail.com with ESMTPSA id 92sm534986uav.9.2021.11.19.11.56.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 19 Nov 2021 11:56:28 -0800 (PST) Message-ID: Date: Fri, 19 Nov 2021 16:56:25 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Subject: Re: Fwd: [PATCH v5 00/22] Some rtld-audit fixes Content-Language: en-US To: Florian Weimer , Jonathon Anderson Cc: John Mellor-Crummey , libc-alpha@sourceware.org, "Mark W. Krentel" , Xiaozhu Meng References: <0D3F0C5F-2586-42F9-916D-2F327432AF13@rice.edu> <87bl2i345m.fsf@oldenburg.str.redhat.com> <87ee7c9coo.fsf@oldenburg.str.redhat.com> From: Adhemerval Zanella In-Reply-To: <87ee7c9coo.fsf@oldenburg.str.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Nov 2021 19:56:32 -0000 On 19/11/2021 16:18, Florian Weimer via Libc-alpha wrote: > * Jonathon Anderson: > >>>> Right now, we >>>> only require the program headers which we can obtain from >>>> getauxval(AT_PHDR), however this technique has questionable >>>> portability and robustness (getauxval returns an unsigned long, not a >>>> pointer). > >>> A glibc port to an architecture where a long value cannot hold all >>> pointer values will have to provide an alternative interface similar to >>> getauxval, but that returns pointer values. > >> I would go one step farther and say getauxval is already broken for >> any 64-bit architecture, unsigned long is only required to support 32 >> bits as per the C standards. One of my greater fears is that some >> exotic compiler will cleverly allocate only 4 bytes of stack space for >> the return value, and we wouldn't know except for a subtle bug >> (dependent on optimization flags!) that crashes our entire tool with >> SEGVs in the auditor (where GDB doesn't give properly unwound call >> stacks). > > If ported to such an architecture, glibc would need several changes to > accomodate this. Newer architectures take this into account and do not > do funny things. But Morello, as a capabilities-based architecture, > does not have this luxury, so they have to do something about this > interface. But that is (still) an outlier. > > I think the important point is that glibc interfaces do not need to be > fully API-compatible with future architecture requirements. We can > change APIs for future ports. > >>> Of course that's not the >>> only interface with this problem (ElfW(Addr) is an integer as well). > >> AFAICT ElfW(Addr) is fine, it should always be an integer large enough >> to store a pointer on the host architecture (i.e. a uintptr_t). Unless >> I missed some specific arch where this doesn't work out to be the >> case? > > Morello and other capabilities-based architectures. Pointers need to > pointers there. Weird architectures do not have uintptr_t. > >>> makes the Morello glibc port quite interesting. So I think *something* >>> like getauxval (AT_PHDR) will always be available, with pretty much >>> identical semantics. >>> >>>> From an outside perspective the current l_addr semantic is fairly >>>> undocumented, the dladdr and dlinfo man pages define it vaguely as >>>> the "difference between the address in the ELF file and the address >>>> in memory." That sounds (to me at least) like l_addr should point to >>>> byte 0 in the file (the ELF header), and that seems to be correct in >>>> all but the non-PIE case. >>> I have struggled with this in the past. I agree that it is confusing. >>> l_addr is the offset between virtual addresses in the program header of >>> the ELF object and the actual addresses in the process image. This >>> offset happens to be 0 for ET_EXEC objects, and only there. > >> This is a much clearer description of the semantic, it would be very >> helpful the man pages used that sentence (or one like it) wherever the >> l_addr value is exposed in the API (link_map->l_addr and >> dl_phdr_info->dlpi_addr). It would also be very helpful if >> Dl_info.dli_fbase was clearly documented as *not* l_addr but instead >> byte 0/ELF header in the process image. > > I've made a note to update the manual pages. > >> That does sound like the "correct" way out, but dl_iterate_phdr >> operates on the caller's namespace so one would need to inject a shim >> library to do the actual call. > > Ugh, you are right. It means we currently can't unwind across dlmopen > boundaries. > > So please use getauxval (AT_PHDR) for now. It is fully portable across > all present glibc targets. > >>>> dladdr gets its value from link_map->l_map_start instead of l_addr, >>>> so the semantic we want is already present in a private field. It >>>> seems to me these two fields could be swapped with little issue, if >>>> altering the public semantic is not acceptable we could also be sated >>>> if l_map_start was made public. >>> Applications which know about the current semantics of l_addr will >>> break, though. l_addr is also exposed to debuggers via the _r_debug >>> interface. I really do not think we can make changes to l_addr. >>> We have a similar issue around l_name being "" for the main program, and >>> unfortuantely I will have to argue quite strongly against changing that. > >> Is adding new public fields completely off the table? > > To struct link_map? We could probably pull it off, but it would be > years until such a change will be in the hands of the users. There is > an internal structure that overlaps with the public struct link_map, > and some applications poke at the private bits at fixed offsets. > > We've started not to strip ld.so downstream, so that these applications > can switch to DWARF data to avoid dependencies on fixed offsets, but > that has been a very recent change. > >> If I can humor the impossible for a few moments longer, I personally >> have a difficult time believing that anyone actually uses >> link_map->l_addr or link_map->l_name in a way that would break by >> changing their semantics for the main executable: > >>  - The documentation hasn't improved for years so there can't be many >> users that care about (or even noticed) this case in particular. > > "" for the main executable is widely known. Usually code uses it to > implement a fallback on argv[0] or /proc/self/exe, though. There are still the issue where audit interface does not have direct access to argv[0] from the audited process and '/proc' might also not be accessible. I am still not convinced that provided argv[0] for l_name for main executable is worse than "", specially because the fallback might not work. > > Changing l_addr will break the libgcc unwinder. It uses l_addr to > relocate the program header (see the code I quoted previously). Not > everyone uses the platform unwinder, and the libgcc unwinder is > sometimes linked statically. This is different from the l_name change: > The l_addr would definitely cause widespread breakage. > >>  - Every use case I can think of for obtaining a link_map from the dl* >> functions (dlinfo and dladdr1) will either already have the special >> handling, or won't operate on the main executable, or likely won't opt >> to use l_addr (vs. dlsym or dli_fbase) or l_name (vs. dli_fname). > > Some special-case the main executable based on l_name, I expect, which > is why I'm so reluctant to change l_name. The GDB comment is actually > hinting strongly towards a "" convention (that Solaris broke). So I take that Solaris does provide the application name to l_name? And what kind of breakage it has done on gdb?