From patchwork Wed Nov 7 20:34:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "H.J. Lu" X-Patchwork-Id: 30065 Received: (qmail 60682 invoked by alias); 7 Nov 2018 20:35:29 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 60660 invoked by uid 89); 7 Nov 2018 20:35:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=ill-formed, 619, illformed X-HELO: mail-ot1-f51.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=W1fdOut1CkEMweE4Gz2B/MZ9P+1Udhi1Mn5qqTHjpaU=; b=okWXyCdTL7ikomFFG8kWAusq883S+NuyJrkvOMt83QFOAFVRQAvlA4Qpv/n8h/r+0X 9O1wwuqLXTC/mk5UXuww6rzpX4zz06PiP32VTQDqNejotYwxek79mC8SBK3ifBYhXo6+ 6QOfo2uFowiOezk69P1m824ZbTcAJYzLrVpPvaxPu7CocbyBYgMJBMQ0gv9EPhR6TfXh JGNd0LMYoskYmJC9GXqpop4bK2YCM4QCozVF44rGW2/FEsWZL9H/QIw7gfiqBTcjZrmW cDGeSV/ddiVZYkhxWkl35nDckDc1YT/pzZZXCs2s2k2C6kHGY209MRGSfBnVhsyaROW6 D4YQ== MIME-Version: 1.0 References: <878t24g2mb.fsf@oldenburg.str.redhat.com> In-Reply-To: <878t24g2mb.fsf@oldenburg.str.redhat.com> From: "H.J. Lu" Date: Wed, 7 Nov 2018 12:34:47 -0800 Message-ID: Subject: Re: V2 [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509] To: Florian Weimer Cc: GNU C Library On Wed, Nov 7, 2018 at 10:32 AM Florian Weimer wrote: > > * H. J. Lu: > > > The older linker treats .note.gnu.property section as a generic note > > section and just concatenates all .note.gnu.property sections from the > > inputs to the output. When the older linker is used to created the > > program on CET-enabled OS, the generated output has .note.gnu.property > > section with multiple NT_GNU_PROPERTY_TYPE_0 notes whose IBT and SHSTK > > enable bits are set even if the program isn't CET enabled. Such program > > will crash on CET-enabled machines. This patch updates the note parser: > > > > 1. Skip note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed. > > 2. Check multiple NT_GNU_PROPERTY_TYPE_0 notes. > > > > [BZ #23509] > > * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Skip > > note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed. > > Update the l_cet field when processing NT_GNU_PROPERTY_TYPE_0 note. > > Check multiple NT_GNU_PROPERTY_TYPE_0 notes. > > * sysdeps/x86/link_map.h (l_cet): Expand to 3 bits, Add > > lc_unknown. > > I would like to move this forward. > > I think the code is okay, as long as we are confident that older linkers > will not produce multiple PT_NOTE segments, each containing one > NT_GNU_PROPERTY_TYPE_0 note. Maybe this should be mentioned in the > commit message (assuming that you agree that my understanding of the > code is correct). > Like this? From b3d82373cb499ceaceafe10b07dd1492c18484c2 Mon Sep 17 00:00:00 2001 From: "H.J. Lu" Date: Fri, 10 Aug 2018 18:43:22 -0700 Subject: [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509] The older linker treats .note.gnu.property section as a generic note section and just concatenates all input .note.gnu.property sections into a single output .note.gnu.property section without merging them. When the older linker is used to created the program on CET-enabled OS, the linker output has a single .note.gnu.property section with multiple NT_GNU_PROPERTY_TYPE_0 notes, some of which have IBT and SHSTK enable bits set even if the program isn't CET enabled. Such programs will crash on CET-enabled machines. This patch updates the note parser: 1. Skip note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed. 2. Check multiple NT_GNU_PROPERTY_TYPE_0 notes. [BZ #23509] * sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Skip note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed. Update the l_cet field when processing NT_GNU_PROPERTY_TYPE_0 note. Check multiple NT_GNU_PROPERTY_TYPE_0 notes. * sysdeps/x86/link_map.h (l_cet): Expand to 3 bits, Add lc_unknown. --- sysdeps/x86/dl-prop.h | 51 +++++++++++++++++++++++++++++++++--------- sysdeps/x86/link_map.h | 9 ++++---- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h index 26c3131ac5..9ab890d12b 100644 --- a/sysdeps/x86/dl-prop.h +++ b/sysdeps/x86/dl-prop.h @@ -49,6 +49,10 @@ _dl_process_cet_property_note (struct link_map *l, const ElfW(Addr) align) { #if CET_ENABLED + /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before. */ + if (l->l_cet != lc_unknown) + return; + /* The NT_GNU_PROPERTY_TYPE_0 note must be aliged to 4 bytes in 32-bit objects and to 8 bytes in 64-bit objects. Skip notes with incorrect alignment. */ @@ -57,6 +61,9 @@ _dl_process_cet_property_note (struct link_map *l, const ElfW(Addr) start = (ElfW(Addr)) note; + unsigned int feature_1 = 0; + unsigned int last_type = 0; + while ((ElfW(Addr)) (note + 1) - start < size) { /* Find the NT_GNU_PROPERTY_TYPE_0 note. */ @@ -64,10 +71,18 @@ _dl_process_cet_property_note (struct link_map *l, && note->n_type == NT_GNU_PROPERTY_TYPE_0 && memcmp (note + 1, "GNU", 4) == 0) { + /* Stop if we see more than one GNU property note which may + be generated by the older linker. */ + if (l->l_cet != lc_unknown) + return; + + /* Check CET status now. */ + l->l_cet = lc_none; + /* Check for invalid property. */ if (note->n_descsz < 8 || (note->n_descsz % sizeof (ElfW(Addr))) != 0) - break; + return; /* Start and end of property array. */ unsigned char *ptr = (unsigned char *) (note + 1) + 4; @@ -78,9 +93,15 @@ _dl_process_cet_property_note (struct link_map *l, unsigned int type = *(unsigned int *) ptr; unsigned int datasz = *(unsigned int *) (ptr + 4); + /* Property type must be in ascending order. */ + if (type < last_type) + return; + ptr += 8; if ((ptr + datasz) > ptr_end) - break; + return; + + last_type = type; if (type == GNU_PROPERTY_X86_FEATURE_1_AND) { @@ -89,14 +110,18 @@ _dl_process_cet_property_note (struct link_map *l, we stop the search regardless if its size is correct or not. There is no point to continue if this note is ill-formed. */ - if (datasz == 4) - { - unsigned int feature_1 = *(unsigned int *) ptr; - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT)) - l->l_cet |= lc_ibt; - if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)) - l->l_cet |= lc_shstk; - } + if (datasz != 4) + return; + + feature_1 = *(unsigned int *) ptr; + + /* Keep searching for the next GNU property note + generated by the older linker. */ + break; + } + else if (type > GNU_PROPERTY_X86_FEATURE_1_AND) + { + /* Stop since property type is in ascending order. */ return; } @@ -112,6 +137,12 @@ _dl_process_cet_property_note (struct link_map *l, + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz, align)); } + + /* We get here only if there is one or no GNU property note. */ + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT)) + l->l_cet |= lc_ibt; + if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK)) + l->l_cet |= lc_shstk; #endif } diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h index ef1206a9d2..9367ed0889 100644 --- a/sysdeps/x86/link_map.h +++ b/sysdeps/x86/link_map.h @@ -19,8 +19,9 @@ /* If this object is enabled with CET. */ enum { - lc_none = 0, /* Not enabled with CET. */ - lc_ibt = 1 << 0, /* Enabled with IBT. */ - lc_shstk = 1 << 1, /* Enabled with STSHK. */ + lc_unknown = 0, /* Unknown CET status. */ + lc_none = 1 << 0, /* Not enabled with CET. */ + lc_ibt = 1 << 1, /* Enabled with IBT. */ + lc_shstk = 1 << 2, /* Enabled with STSHK. */ lc_ibt_and_shstk = lc_ibt | lc_shstk /* Enabled with both. */ - } l_cet:2; + } l_cet:3; -- 2.19.1