From patchwork Mon Mar 27 11:27:40 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alan Modra X-Patchwork-Id: 75350 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp1434116vqo; Mon, 27 Mar 2023 04:27:56 -0700 (PDT) X-Google-Smtp-Source: AKy350ZyfGCADw8RVEFqKIL4hFSIYexh7oHFsu1+ZCPn7SRMJ9l0xHfQjwBstovn5zjSJboQg0Q8 X-Received: by 2002:aa7:c847:0:b0:4fa:9a9d:94e with SMTP id g7-20020aa7c847000000b004fa9a9d094emr11041454edt.2.1679916475884; Mon, 27 Mar 2023 04:27:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1679916475; cv=none; d=google.com; s=arc-20160816; b=a4Vqimj1MHURRo9ZuadzQIRTg7oDHNi6sQxGLudwGOq5AmfMarI7yrM+9N13CRfGks 2aYo4PkFTQG3kITTyusOjiSuCd03nyExf+kVojdbh2okmrvIG2kuXdGXMfoD5rNuAFAY vW1GSQpeiwteXk/skDY5gcfj1Y0ugX8q0WGhx5KPNs2to3hFRF5hgt37OeHd8XAv6G5D nTNzF5tXuHUK4j6WGxHkLhEVXVRBm6HC9BvuUie5NAcC4z484QMazlk9Pn3Iq2/pcyAU j5EQM5JaZecq5G3l8YU3amCWYUwRMMbIT6DVDpJ3KbshbW1WaCh4S13yjntAOo1ArtRn rSNw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:reply-to:from:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence :content-disposition:mime-version:message-id:subject:to:date :dmarc-filter:delivered-to:dkim-signature:dkim-filter; bh=QW8ad9MN/vxYJSKMLsDDgu2pD2cx+TfjFuqokNaF248=; b=Q9FNm+PAfHgNer8kBCvhUv11RM8S/Hr40oeQHlkDacujmT7SJVCADqUqVqYLPKWE+Y NRj1XrLaSTZ4ZngwiRLENO4+hdxvCvlD8lXml5blK84URSs0GIiWb7XX2RExGblDZTdT PcCwxj1eMAZ5hBw4dUDPWCaJtbB2vxeO4hLr7kukYkztLVmXQw0wZyfZyF9o02+V6ALx ox8SMNv7Kr7i6A2TNdIxyDnVWu1+x6iB+D+0jXW8GnRIoFTBQzMWYDMMHdvuoDWco5+e nw6PbWrdJ0t2IK1hCLt/Wt5hOEb0aW7EZ9QrrJEcDlHvHjQ9gz9NaatsY6JLkEkWGi8K myBg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=uekutyAd; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id u16-20020a056402111000b004af515d5657si25879293edv.61.2023.03.27.04.27.55 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Mar 2023 04:27:55 -0700 (PDT) Received-SPF: pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=uekutyAd; spf=pass (google.com: domain of binutils-bounces+ouuuleilei=gmail.com@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="binutils-bounces+ouuuleilei=gmail.com@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8E8613858407 for ; Mon, 27 Mar 2023 11:27:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8E8613858407 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1679916473; bh=QW8ad9MN/vxYJSKMLsDDgu2pD2cx+TfjFuqokNaF248=; h=Date:To:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=uekutyAdJmFSNxCwnHZyStyBWmnz1dWDNPbApBF8z34vZrwkfOGBfQiWC3eBod6CQ dLBsQGTs0jSRtifxPay4QDvfAeBSnDaJqY9dW/ZMpizb/uAUkgqy7gE07C7HY/6Vil vG6CSXAf6R9WBzedWxG7qw7EAfBIbNle/pFhdUyM= X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from mail-pl1-x632.google.com (mail-pl1-x632.google.com [IPv6:2607:f8b0:4864:20::632]) by sourceware.org (Postfix) with ESMTPS id 5C1E33858D39 for ; Mon, 27 Mar 2023 11:27:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5C1E33858D39 Received: by mail-pl1-x632.google.com with SMTP id w4so8092339plg.9 for ; Mon, 27 Mar 2023 04:27:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679916463; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QW8ad9MN/vxYJSKMLsDDgu2pD2cx+TfjFuqokNaF248=; b=6jDIHeFMn/CVLMB5IHwZ2egfgzegOhPvFVfM3+K0+bWBQmjMkQZGm4T42ouoOZZe+V ov6bYkVHXrmRKBGFwBOuhqTD4A/iR3VvazR1pA40JXCwWqcT+RwbW1Vu6oNYSshbaYTH zHIuPfUyJ269RjwI6zfIy1nX8MHSBj1HlXbAfx9OXGuXwCW8e9S8x6fOsvnlhuAJVngV bDaIDr6+TNgF+d1XQaUfrvmnjotKbo5JJ+SrDnFFnhAfcI7pN5YP7ihfMAzHS6LhCnJF kTnHffBJkYk/lBYs8Zg1WCtd6aHSYOxTTgrOPBR01NUS5G3KU0jG+vYKM6CGlMFyXOyB iy2A== X-Gm-Message-State: AO0yUKXbQYcAvemkeydjWOis6J0Qy9k0m67L8F9A9Sc3wyXraCKGa7aY h06pZRnAzkEkiWkZE6DN0HhuQOr05gQ= X-Received: by 2002:a05:6a20:4987:b0:d5:3818:6427 with SMTP id fs7-20020a056a20498700b000d538186427mr9826148pzb.9.1679916463179; Mon, 27 Mar 2023 04:27:43 -0700 (PDT) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:1b0:13cb:4f4d:735]) by smtp.gmail.com with ESMTPSA id y17-20020aa78051000000b006288ca3cadfsm1161018pfm.35.2023.03.27.04.27.42 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Mar 2023 04:27:42 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id A3D1F11415C5; Mon, 27 Mar 2023 21:57:40 +1030 (ACDT) Date: Mon, 27 Mar 2023 21:57:40 +1030 To: binutils@sourceware.org Subject: coffgrok access of u.auxent.x_sym.x_tagndx.p Message-ID: MIME-Version: 1.0 Content-Disposition: inline X-Spam-Status: No, score=-3034.2 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP 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: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Alan Modra via Binutils From: Alan Modra Reply-To: Alan Modra Errors-To: binutils-bounces+ouuuleilei=gmail.com@sourceware.org Sender: "Binutils" X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1761520098465977729?= X-GMAIL-MSGID: =?utf-8?q?1761520098465977729?= u.auxent.x_sym.x_tagndx is a union. The p field is only valid when fix_tag is set. This patch fixes code in coffgrok.c that accessed the field without first checking fix_tag, and removes a whole lot of code validating bogus pointers to prevent segfaults (which no longer happen, I checked the referenced PR 17512 testcases). The patch also documents this in the fix_tag comment, makes is_sym a bitfield, and sorts the selecter fields a little. bfd/ * coffcode.h (combined_entry_type): Make is_sym a bitfield. Sort and comment on union selectors. * libcoff.h: Regenerate. binutils/ * coffgrok.c (do_type): Make aux a combined_entry_type. Test fix_tag before accessing u.auxent.x_sym.x_tagndx.p. Remove now unnecessary pointer bounds checking. diff --git a/bfd/coffcode.h b/bfd/coffcode.h index bf55d83530d..d4a2a5c3d62 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -294,27 +294,30 @@ CODE_FRAGMENT .typedef struct coff_ptr_struct .{ . {* Remembers the offset from the first symbol in the file for -. this symbol. Generated by coff_renumber_symbols. *} +. this symbol. Generated by coff_renumber_symbols. *} . unsigned int offset; . -. {* Should the value of this symbol be renumbered. Used for -. XCOFF C_BSTAT symbols. Set by coff_slurp_symbol_table. *} -. unsigned int fix_value : 1; +. {* Selects between the elements of the union below. *} +. unsigned int is_sym : 1; . -. {* Should the tag field of this symbol be renumbered. -. Created by coff_pointerize_aux. *} +. {* Selects between the elements of the x_sym.x_tagndx union. If set, +. p is valid and the field will be renumbered. *} . unsigned int fix_tag : 1; . -. {* Should the endidx field of this symbol be renumbered. -. Created by coff_pointerize_aux. *} +. {* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx +. union. If set, p is valid and the field will be renumbered. *} . unsigned int fix_end : 1; . -. {* Should the x_csect.x_scnlen field be renumbered. -. Created by coff_pointerize_aux. *} +. {* Selects between the elements of the x_csect.x_scnlen union. If set, +. p is valid and the field will be renumbered. *} . unsigned int fix_scnlen : 1; . -. {* Fix up an XCOFF C_BINCL/C_EINCL symbol. The value is the -. index into the line number entries. Set by coff_slurp_symbol_table. *} +. {* If set, u.syment.n_value contains a pointer to a symbol. The final +. value will be the offset field. Used for XCOFF C_BSTAT symbols. *} +. unsigned int fix_value : 1; +. +. {* If set, u.syment.n_value is an index into the line number entries. +. Used for XCOFF C_BINCL/C_EINCL symbols. *} . unsigned int fix_line : 1; . . {* The container for the symbol structure as read and translated @@ -325,9 +328,6 @@ CODE_FRAGMENT . struct internal_syment syment; . } u; . -. {* Selector for the union above. *} -. bool is_sym; -. . {* An extra pointer which can used by format based on COFF (like XCOFF) . to provide extra information to their backend. *} . void *extrap; diff --git a/bfd/libcoff.h b/bfd/libcoff.h index b7a4f677411..b58ee8adedf 100644 --- a/bfd/libcoff.h +++ b/bfd/libcoff.h @@ -633,27 +633,30 @@ extern bool _bfd_ppc_xcoff_relocate_section typedef struct coff_ptr_struct { /* Remembers the offset from the first symbol in the file for - this symbol. Generated by coff_renumber_symbols. */ + this symbol. Generated by coff_renumber_symbols. */ unsigned int offset; - /* Should the value of this symbol be renumbered. Used for - XCOFF C_BSTAT symbols. Set by coff_slurp_symbol_table. */ - unsigned int fix_value : 1; + /* Selects between the elements of the union below. */ + unsigned int is_sym : 1; - /* Should the tag field of this symbol be renumbered. - Created by coff_pointerize_aux. */ + /* Selects between the elements of the x_sym.x_tagndx union. If set, + p is valid and the field will be renumbered. */ unsigned int fix_tag : 1; - /* Should the endidx field of this symbol be renumbered. - Created by coff_pointerize_aux. */ + /* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx + union. If set, p is valid and the field will be renumbered. */ unsigned int fix_end : 1; - /* Should the x_csect.x_scnlen field be renumbered. - Created by coff_pointerize_aux. */ + /* Selects between the elements of the x_csect.x_scnlen union. If set, + p is valid and the field will be renumbered. */ unsigned int fix_scnlen : 1; - /* Fix up an XCOFF C_BINCL/C_EINCL symbol. The value is the - index into the line number entries. Set by coff_slurp_symbol_table. */ + /* If set, u.syment.n_value contains a pointer to a symbol. The final + value will be the offset field. Used for XCOFF C_BSTAT symbols. */ + unsigned int fix_value : 1; + + /* If set, u.syment.n_value is an index into the line number entries. + Used for XCOFF C_BINCL/C_EINCL symbols. */ unsigned int fix_line : 1; /* The container for the symbol structure as read and translated @@ -664,9 +667,6 @@ typedef struct coff_ptr_struct struct internal_syment syment; } u; - /* Selector for the union above. */ - bool is_sym; - /* An extra pointer which can used by format based on COFF (like XCOFF) to provide extra information to their backend. */ void *extrap; diff --git a/binutils/coffgrok.c b/binutils/coffgrok.c index f2676e4c275..fa01203ab5e 100644 --- a/binutils/coffgrok.c +++ b/binutils/coffgrok.c @@ -341,7 +341,7 @@ static struct coff_type * do_type (unsigned int i) { struct internal_syment *sym; - union internal_auxent *aux; + combined_entry_type *aux; struct coff_type *res = (struct coff_type *) xmalloc (sizeof (struct coff_type)); int type; int which_dt = 0; @@ -357,7 +357,7 @@ do_type (unsigned int i) if (sym->n_numaux == 0 || i >= rawcount -1 || rawsyms[i + 1].is_sym) aux = NULL; else - aux = &rawsyms[i + 1].u.auxent; + aux = &rawsyms[i + 1]; type = sym->n_type; @@ -374,7 +374,7 @@ do_type (unsigned int i) res->type = coff_secdef_type; if (aux == NULL) fatal (_("Section definition needs a section length")); - res->size = aux->x_scn.x_scnlen; + res->size = aux->u.auxent.x_scn.x_scnlen; /* PR 17512: file: 081c955d. Fill in the asecdef structure as well. */ @@ -426,26 +426,9 @@ do_type (unsigned int i) if (aux == NULL) fatal (_("Aggregate definition needs auxiliary information")); - if (aux->x_sym.x_tagndx.p) + if (aux->fix_tag) { - unsigned int idx; - - /* PR 17512: file: e72f3988. */ - if (aux->x_sym.x_tagndx.l < 0 || aux->x_sym.x_tagndx.p < rawsyms) - { - non_fatal (_("Invalid tag index %#lx encountered"), aux->x_sym.x_tagndx.l); - idx = 0; - } - else - idx = INDEXOF (aux->x_sym.x_tagndx.p); - - if (idx >= rawcount) - { - if (rawcount == 0) - fatal (_("Symbol index %u encountered when there are no symbols"), idx); - non_fatal (_("Invalid symbol index %u encountered"), idx); - idx = 0; - } + unsigned int idx = INDEXOF (aux->u.auxent.x_sym.x_tagndx.p); /* Referring to a struct defined elsewhere. */ res->type = coff_structref_type; @@ -461,7 +444,7 @@ do_type (unsigned int i) res->u.astructdef.elements = empty_scope (); res->u.astructdef.idx = 0; res->u.astructdef.isstruct = (type & 0xf) == T_STRUCT; - res->size = aux->x_sym.x_misc.x_lnsz.x_size; + res->size = aux->u.auxent.x_sym.x_misc.x_lnsz.x_size; } } else @@ -475,13 +458,10 @@ do_type (unsigned int i) case T_ENUM: if (aux == NULL) fatal (_("Enum definition needs auxiliary information")); - if (aux->x_sym.x_tagndx.p) + if (aux->fix_tag) { - unsigned int idx = INDEXOF (aux->x_sym.x_tagndx.p); + unsigned int idx = INDEXOF (aux->u.auxent.x_sym.x_tagndx.p); - /* PR 17512: file: 1ef037c7. */ - if (idx >= rawcount) - fatal (_("Invalid enum symbol index %u encountered"), idx); /* Referring to a enum defined elsewhere. */ res->type = coff_enumref_type; res->u.aenumref.ref = tindex[idx]; @@ -497,7 +477,7 @@ do_type (unsigned int i) last_enum = res; res->type = coff_enumdef_type; res->u.aenumdef.elements = empty_scope (); - res->size = aux->x_sym.x_misc.x_lnsz.x_size; + res->size = aux->u.auxent.x_sym.x_misc.x_lnsz.x_size; } break; case T_MOE: @@ -519,7 +499,7 @@ do_type (unsigned int i) if (aux == NULL) fatal (_("Array definition needs auxiliary information")); els = (dimind < DIMNUM - ? aux->x_sym.x_fcnary.x_ary.x_dimen[dimind] + ? aux->u.auxent.x_sym.x_fcnary.x_ary.x_dimen[dimind] : 0); ++dimind;