From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id EDC773858D39; Mon, 27 Mar 2023 11:29:25 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EDC773858D39 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: bfd-cvs@sourceware.org Subject: [binutils-gdb] coffgrok access of u.auxent.x_sym.x_tagndx.p X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: 92479281c4621e8d71565f76b879e36bf92b0b18 X-Git-Newrev: 695c322803476e92e1566c90470b6bb737a40514 Message-Id: <20230327112925.EDC773858D39@sourceware.org> Date: Mon, 27 Mar 2023 11:29:25 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Mar 2023 11:29:26 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D695c32280347= 6e92e1566c90470b6bb737a40514 commit 695c322803476e92e1566c90470b6bb737a40514 Author: Alan Modra Date: Sun Mar 26 19:26:46 2023 +1030 coffgrok access of u.auxent.x_sym.x_tagndx.p =20 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. =20 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: --- bfd/coffcode.h | 30 +++++++++++++++--------------- bfd/libcoff.h | 30 +++++++++++++++--------------- binutils/coffgrok.c | 40 ++++++++++------------------------------ 3 files changed, 40 insertions(+), 60 deletions(-) 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; =20 - /* 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; =20 - /* 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; =20 - /* 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; =20 - /* 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; =20 - /* 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; =20 /* The container for the symbol structure as read and translated @@ -664,9 +667,6 @@ typedef struct coff_ptr_struct struct internal_syment syment; } u; =20 - /* 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 =3D (struct coff_type *) xmalloc (sizeof (struct c= off_type)); int type; int which_dt =3D 0; @@ -357,7 +357,7 @@ do_type (unsigned int i) if (sym->n_numaux =3D=3D 0 || i >=3D rawcount -1 || rawsyms[i + 1].is_sy= m) aux =3D NULL; else - aux =3D &rawsyms[i + 1].u.auxent; + aux =3D &rawsyms[i + 1]; =20 type =3D sym->n_type; =20 @@ -374,7 +374,7 @@ do_type (unsigned int i) res->type =3D coff_secdef_type; if (aux =3D=3D NULL) fatal (_("Section definition needs a section length")); - res->size =3D aux->x_scn.x_scnlen; + res->size =3D aux->u.auxent.x_scn.x_scnlen; =20 /* PR 17512: file: 081c955d. Fill in the asecdef structure as well. */ @@ -426,26 +426,9 @@ do_type (unsigned int i) if (aux =3D=3D NULL) fatal (_("Aggregate definition needs auxiliary information")); =20 - 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_tagnd= x.l); - idx =3D 0; - } - else - idx =3D INDEXOF (aux->x_sym.x_tagndx.p); - - if (idx >=3D rawcount) - { - if (rawcount =3D=3D 0) - fatal (_("Symbol index %u encountered when there are no symbols"), i= dx); - non_fatal (_("Invalid symbol index %u encountered"), idx); - idx =3D 0; - } + unsigned int idx =3D INDEXOF (aux->u.auxent.x_sym.x_tagndx.p); =20 /* Referring to a struct defined elsewhere. */ res->type =3D coff_structref_type; @@ -461,7 +444,7 @@ do_type (unsigned int i) res->u.astructdef.elements =3D empty_scope (); res->u.astructdef.idx =3D 0; res->u.astructdef.isstruct =3D (type & 0xf) =3D=3D T_STRUCT; - res->size =3D aux->x_sym.x_misc.x_lnsz.x_size; + res->size =3D 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 =3D=3D NULL) fatal (_("Enum definition needs auxiliary information")); - if (aux->x_sym.x_tagndx.p) + if (aux->fix_tag) { - unsigned int idx =3D INDEXOF (aux->x_sym.x_tagndx.p); + unsigned int idx =3D INDEXOF (aux->u.auxent.x_sym.x_tagndx.p); =20 - /* PR 17512: file: 1ef037c7. */ - if (idx >=3D rawcount) - fatal (_("Invalid enum symbol index %u encountered"), idx); /* Referring to a enum defined elsewhere. */ res->type =3D coff_enumref_type; res->u.aenumref.ref =3D tindex[idx]; @@ -497,7 +477,7 @@ do_type (unsigned int i) last_enum =3D res; res->type =3D coff_enumdef_type; res->u.aenumdef.elements =3D empty_scope (); - res->size =3D aux->x_sym.x_misc.x_lnsz.x_size; + res->size =3D 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 =3D=3D NULL) fatal (_("Array definition needs auxiliary information")); els =3D (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); =20 ++dimind;