From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yb1-xb35.google.com (mail-yb1-xb35.google.com [IPv6:2607:f8b0:4864:20::b35]) by sourceware.org (Postfix) with ESMTPS id 186AA3857358 for ; Sun, 12 Jun 2022 21:35:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 186AA3857358 Received: by mail-yb1-xb35.google.com with SMTP id p13so7126329ybm.1 for ; Sun, 12 Jun 2022 14:35:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0nTWh7leB3K3uruQAEi+IEs5t5jKXuoelbDY9V8U4sU=; b=ao9HE6f/LvNI0kJxzIKXfe3qa9+bOC+ojoBs9P0Nyccrk2iTl64FffOHiF1zFDtzM9 qxhQkfjqyj5vmPcr80aJWiR7aZxEVwKdICV660UQM8226yfUf5Znu+tdQrPLlYnc09f4 Qbo3AqNTLCA/B12AjKDq9ELits2vNyNbbZmVCVbSQWbRFLSFVIigSsh7WTFNw/4ipD9p FDraqxfvzySQ5wMarG84N+ppCsMlWOVs9P7FsWrDK+FxXEqUCy6oQs3bnRSl3O0k1Zdw kG+52GLjy/dCr3s8LrK8OIQiQy3hdwC5EjTFgenIcA3Jq2JTRaKV344cLuXU3j9KStGv DFDg== X-Gm-Message-State: AOAM530uUu3oekqs+v8wPaMFw0lpVYCg878UG6e4v9PZXvXRwRQnkrF1 3nM6o/ZDYK86se1SiepQyMjrMjXTW9nQeSlALWiSXA== X-Google-Smtp-Source: ABdhPJxk0B47yWGwu69B2ftYLz22J1JrzUUD0/f9wcmxOMVrvVxPubw1JgKxU5jSM7pexv/T3DDiU5rlUG8osSWLSK8= X-Received: by 2002:a25:6b09:0:b0:65d:1219:5e32 with SMTP id g9-20020a256b09000000b0065d12195e32mr53320484ybc.233.1655069719146; Sun, 12 Jun 2022 14:35:19 -0700 (PDT) MIME-Version: 1.0 References: <20220608193701.3211265-1-maskray@google.com> In-Reply-To: From: Fangrui Song Date: Sun, 12 Jun 2022 14:35:08 -0700 Message-ID: Subject: Re: [PATCH v2] elf: Refine direct extern access diagnostics to protected symbol To: "H.J. Lu" Cc: GNU C Library Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-23.8 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) 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: Sun, 12 Jun 2022 21:35:21 -0000 On Sun, Jun 12, 2022 at 2:24 PM H.J. Lu wrote: > > On Wed, Jun 8, 2022 at 12:37 PM Fangrui Song via Libc-alpha > wrote: > > > > Refine commit 349b0441dab375099b1d7f6909c1742286a67da9: > > > > 1. Copy relocations for extern protected data do not work properly, > > regardless whether GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS is used. > > It makes sense to produce a warning unconditionally. When the defining > > shared object has GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS, report > > an error to satisfy the "no copy relocations" enforcement intended by > > this GNU property. > > > > 2. Non-zero value of an undefined function symbol may break pointer > > equality, but may be benign in many cases (many programs don't take the > > address in the shared object then compare it with the address in the > > executable). Reword the diagnostic to be clearer. > > > > 3. Remove the unneeded condition !(undef_map->l_1_needed & > > GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS). If the executable has > > GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (can only occur in error > > cases), the diagnostic should be emitted as well. > > > > -- > > Changes from v1: > > * Keep 2 as an error in the presence of GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS > > as requested by H.J. Lu. > > --- > > sysdeps/generic/dl-protected.h | 50 ++++++++++++++++++---------------- > > 1 file changed, 27 insertions(+), 23 deletions(-) > > > > diff --git a/sysdeps/generic/dl-protected.h b/sysdeps/generic/dl-protected.h > > index 88cb8ec917..38386b5200 100644 > > --- a/sysdeps/generic/dl-protected.h > > +++ b/sysdeps/generic/dl-protected.h > > @@ -26,29 +26,33 @@ _dl_check_protected_symbol (const char *undef_name, > > const struct link_map *map, > > int type_class) > > { > > - if (undef_map != NULL > > - && undef_map->l_type == lt_executable > > - && !(undef_map->l_1_needed > > - & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) > > - && (map->l_1_needed > > - & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS)) > > - { > > - if ((type_class & ELF_RTYPE_CLASS_COPY)) > > - /* Disallow copy relocations in executable against protected > > - data symbols in a shared object which needs indirect external > > - access. */ > > - _dl_signal_error (0, map->l_name, undef_name, > > - N_("copy relocation against non-copyable protected symbol")); > > - else if (ref->st_value != 0 > > - && ref->st_shndx == SHN_UNDEF > > - && (type_class & ELF_RTYPE_CLASS_PLT)) > > - /* Disallow non-zero symbol values of undefined symbols in > > - executable, which are used as the function pointer, against > > - protected function symbols in a shared object with indirect > > - external access. */ > > - _dl_signal_error (0, map->l_name, undef_name, > > - N_("non-canonical reference to canonical protected function")); > > - } > > + if (undef_map == NULL || undef_map->l_type != lt_executable) > > + return; > > + > > + if (type_class & ELF_RTYPE_CLASS_COPY) > > + /* Disallow copy relocations in executable against protected > > + data symbols in a shared object which needs indirect external > > + access. */ > > + _dl_error_printf ("warning: copy relocation against non-copyable " > > + "protected symbol `%s' in `%s'\n", > > + undef_name, map->l_name); > > + else if ((type_class & ELF_RTYPE_CLASS_PLT) && ref->st_value != 0 > > + && ref->st_shndx == SHN_UNDEF) > > + /* Disallow non-zero symbol values of undefined symbols in > > + executable, which are used as the function pointer, against > > + protected function symbols in a shared object with indirect > > + external access. */ > > + _dl_error_printf ( > > + "warning: direct reference to " > > I think the format convention is > > _dl_error_printf ("warning: direct reference to " The patch matches both Emacs C-M-\ (GNU indentation style) and clang-format. They place the string literal on the continuation line... > > + "protected function `%s' in `%s' may break pointer equality\n", > > + undef_name, map->l_name); > > + else > > + return; > > + > > + if (map->l_1_needed & GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS) > > + _dl_signal_error ( > > + 0, map->l_name, undef_name, > > _dl_signal_error (0, map->l_name, undef_name, > > > + N_ ("error due to GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS")); > > } > > > > #endif /* _DL_PROTECTED_H */ > > -- > > 2.36.1.255.ge46751e96f-goog > > > > > -- > H.J.