From 695c322803476e92e1566c90470b6bb737a40514 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Sun, 26 Mar 2023 19:26:46 +1030 Subject: coffgrok access of u.auxent.x_sym.x_tagndx.p 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. --- 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 bf55d83..d4a2a5c 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 b7a4f67..b58ee8a 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 f2676e4..fa01203 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; -- cgit v1.1