Toolchain/binutils-gdb
1
0
Fork 0
forked from Imagelibrary/binutils-gdb
Code Pull Requests Activity

coffgrok access of u.auxent.x_sym.x_tagndx.p

Browse Source 
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.
This commit is contained in:
Alan Modra
2023-03-26 19:26:46 +10:30
parent 92479281c4
commit 695c322803
3 changed files with 40 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
bfd/coffcode.h
View File

@@ -297,24 +297,27 @@ CODE_FRAGMENT
. this symbol. Generated by coff_renumber_symbols. *} . this symbol. Generated by coff_renumber_symbols. *}
. unsigned int offset; . unsigned int offset;
. .
. {* Should the value of this symbol be renumbered. Used for . {* Selects between the elements of the union below. *}
. XCOFF C_BSTAT symbols. Set by coff_slurp_symbol_table. *} . unsigned int is_sym : 1;
. unsigned int fix_value : 1;
. .
. {* Should the tag field of this symbol be renumbered. . {* Selects between the elements of the x_sym.x_tagndx union. If set,
. Created by coff_pointerize_aux. *} . p is valid and the field will be renumbered. *}
. unsigned int fix_tag : 1; . unsigned int fix_tag : 1;
. .
. {* Should the endidx field of this symbol be renumbered. . {* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx
. Created by coff_pointerize_aux. *} . union. If set, p is valid and the field will be renumbered. *}
. unsigned int fix_end : 1; . unsigned int fix_end : 1;
. .
. {* Should the x_csect.x_scnlen field be renumbered. . {* Selects between the elements of the x_csect.x_scnlen union. If set,
. Created by coff_pointerize_aux. *} . p is valid and the field will be renumbered. *}
. unsigned int fix_scnlen : 1; . unsigned int fix_scnlen : 1;
. .
. {* Fix up an XCOFF C_BINCL/C_EINCL symbol. The value is the . {* If set, u.syment.n_value contains a pointer to a symbol. The final
. index into the line number entries. Set by coff_slurp_symbol_table. *} . 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; . unsigned int fix_line : 1;
. .
. {* The container for the symbol structure as read and translated . {* The container for the symbol structure as read and translated
@@ -325,9 +328,6 @@ CODE_FRAGMENT
. struct internal_syment syment; . struct internal_syment syment;
. } u; . } u;
. .
. {* Selector for the union above. *}
. bool is_sym;
.
. {* An extra pointer which can used by format based on COFF (like XCOFF) . {* An extra pointer which can used by format based on COFF (like XCOFF)
. to provide extra information to their backend. *} . to provide extra information to their backend. *}
. void *extrap; . void *extrap;

28
bfd/libcoff.h
View File

@@ -636,24 +636,27 @@ typedef struct coff_ptr_struct
this symbol. Generated by coff_renumber_symbols. */ this symbol. Generated by coff_renumber_symbols. */
unsigned int offset; unsigned int offset;
/* Should the value of this symbol be renumbered. Used for /* Selects between the elements of the union below. */
XCOFF C_BSTAT symbols. Set by coff_slurp_symbol_table. */ unsigned int is_sym : 1;
unsigned int fix_value : 1;
/* Should the tag field of this symbol be renumbered. /* Selects between the elements of the x_sym.x_tagndx union. If set,
Created by coff_pointerize_aux. */ p is valid and the field will be renumbered. */
unsigned int fix_tag : 1; unsigned int fix_tag : 1;
/* Should the endidx field of this symbol be renumbered. /* Selects between the elements of the x_sym.x_fcnary.x_fcn.x_endndx
Created by coff_pointerize_aux. */ union. If set, p is valid and the field will be renumbered. */
unsigned int fix_end : 1; unsigned int fix_end : 1;
/* Should the x_csect.x_scnlen field be renumbered. /* Selects between the elements of the x_csect.x_scnlen union. If set,
Created by coff_pointerize_aux. */ p is valid and the field will be renumbered. */
unsigned int fix_scnlen : 1; unsigned int fix_scnlen : 1;
/* Fix up an XCOFF C_BINCL/C_EINCL symbol. The value is the /* If set, u.syment.n_value contains a pointer to a symbol. The final
index into the line number entries. Set by coff_slurp_symbol_table. */ 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; unsigned int fix_line : 1;
/* The container for the symbol structure as read and translated /* The container for the symbol structure as read and translated
@@ -664,9 +667,6 @@ typedef struct coff_ptr_struct
struct internal_syment syment; struct internal_syment syment;
} u; } u;
/* Selector for the union above. */
bool is_sym;
/* An extra pointer which can used by format based on COFF (like XCOFF) /* An extra pointer which can used by format based on COFF (like XCOFF)
to provide extra information to their backend. */ to provide extra information to their backend. */
void *extrap; void *extrap;

40
binutils/coffgrok.c
View File

@@ -341,7 +341,7 @@ static struct coff_type *
do_type (unsigned int i) do_type (unsigned int i)
{ {
struct internal_syment *sym; struct internal_syment *sym;
union internal_auxent *aux; combined_entry_type *aux;
struct coff_type *res = (struct coff_type *) xmalloc (sizeof (struct coff_type)); struct coff_type *res = (struct coff_type *) xmalloc (sizeof (struct coff_type));
int type; int type;
int which_dt = 0; 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) if (sym->n_numaux == 0 || i >= rawcount -1 || rawsyms[i + 1].is_sym)
aux = NULL; aux = NULL;
else else
aux = &rawsyms[i + 1].u.auxent; aux = &rawsyms[i + 1];
type = sym->n_type; type = sym->n_type;
@@ -374,7 +374,7 @@ do_type (unsigned int i)
res->type = coff_secdef_type; res->type = coff_secdef_type;
if (aux == NULL) if (aux == NULL)
fatal (_("Section definition needs a section length")); 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. /* PR 17512: file: 081c955d.
Fill in the asecdef structure as well. */ Fill in the asecdef structure as well. */
@@ -426,26 +426,9 @@ do_type (unsigned int i)
if (aux == NULL) if (aux == NULL)
fatal (_("Aggregate definition needs auxiliary information")); fatal (_("Aggregate definition needs auxiliary information"));
if (aux->x_sym.x_tagndx.p) if (aux->fix_tag)
{ {
unsigned int idx; unsigned int idx = INDEXOF (aux->u.auxent.x_sym.x_tagndx.p);
/* 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;
}
/* Referring to a struct defined elsewhere. */ /* Referring to a struct defined elsewhere. */
res->type = coff_structref_type; res->type = coff_structref_type;
@@ -461,7 +444,7 @@ do_type (unsigned int i)
res->u.astructdef.elements = empty_scope (); res->u.astructdef.elements = empty_scope ();
res->u.astructdef.idx = 0; res->u.astructdef.idx = 0;
res->u.astructdef.isstruct = (type & 0xf) == T_STRUCT; 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 else
@@ -475,13 +458,10 @@ do_type (unsigned int i)
case T_ENUM: case T_ENUM:
if (aux == NULL) if (aux == NULL)
fatal (_("Enum definition needs auxiliary information")); 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. */ /* Referring to a enum defined elsewhere. */
res->type = coff_enumref_type; res->type = coff_enumref_type;
res->u.aenumref.ref = tindex[idx]; res->u.aenumref.ref = tindex[idx];
@@ -497,7 +477,7 @@ do_type (unsigned int i)
last_enum = res; last_enum = res;
res->type = coff_enumdef_type; res->type = coff_enumdef_type;
res->u.aenumdef.elements = empty_scope (); 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; break;
case T_MOE: case T_MOE:
@@ -519,7 +499,7 @@ do_type (unsigned int i)
if (aux == NULL) if (aux == NULL)
fatal (_("Array definition needs auxiliary information")); fatal (_("Array definition needs auxiliary information"));
els = (dimind < DIMNUM 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); : 0);
++dimind; ++dimind;