bfd_check_format_matches preserving matches vs. cleanups
It didn't take long for oss-fuzz to find double frees due to a bug in the cleanup logic. It's seen when reading in any alpha-vms object file except when alpha_vms_vec is the default. But alpha_vms_vec is of course the default when building for --target=alpha-dec-vms (and naturally what I used to test the cleanup support since that is the only target with a cleanup that does anything currently). Anyway, the bug is that if bfd_check_format_matches is to preserve a match the cleanup for that match can't be run. Quite obviously that would destroy part of the match state. * format.c (struct bfd_preserve): Add cleanup field. (bfd_preserve_save): Add cleanup param and save. (bfd_preserve_restore): Return cleanup. (bfd_preserve_finish): Call the cleanup for the discarded match. (bfd_check_format_matches): Pass cleanup to bfd_preserve_save, and clear when preserving a match. Restore cleanup too when restoring that match.
This commit is contained in:
parent
478e490a4d
commit
f57140990f
|
@ -1,3 +1,13 @@
|
||||||
|
2020-03-03 Alan Modra <amodra@gmail.com>
|
||||||
|
|
||||||
|
* format.c (struct bfd_preserve): Add cleanup field.
|
||||||
|
(bfd_preserve_save): Add cleanup param and save.
|
||||||
|
(bfd_preserve_restore): Return cleanup.
|
||||||
|
(bfd_preserve_finish): Call the cleanup for the discarded match.
|
||||||
|
(bfd_check_format_matches): Pass cleanup to bfd_preserve_save,
|
||||||
|
and clear when preserving a match. Restore cleanup too when
|
||||||
|
restoring that match.
|
||||||
|
|
||||||
2020-03-02 Alan Modra <amodra@gmail.com>
|
2020-03-02 Alan Modra <amodra@gmail.com>
|
||||||
|
|
||||||
* cisco-core.c (cisco_core_file_p): Return bfd_cleanup.
|
* cisco-core.c (cisco_core_file_p): Return bfd_cleanup.
|
||||||
|
|
24
bfd/format.c
24
bfd/format.c
|
@ -106,6 +106,7 @@ struct bfd_preserve
|
||||||
unsigned int section_id;
|
unsigned int section_id;
|
||||||
struct bfd_hash_table section_htab;
|
struct bfd_hash_table section_htab;
|
||||||
const struct bfd_build_id *build_id;
|
const struct bfd_build_id *build_id;
|
||||||
|
bfd_cleanup cleanup;
|
||||||
};
|
};
|
||||||
|
|
||||||
/* When testing an object for compatibility with a particular target
|
/* When testing an object for compatibility with a particular target
|
||||||
|
@ -118,7 +119,8 @@ struct bfd_preserve
|
||||||
the subset. */
|
the subset. */
|
||||||
|
|
||||||
static bfd_boolean
|
static bfd_boolean
|
||||||
bfd_preserve_save (bfd *abfd, struct bfd_preserve *preserve)
|
bfd_preserve_save (bfd *abfd, struct bfd_preserve *preserve,
|
||||||
|
bfd_cleanup cleanup)
|
||||||
{
|
{
|
||||||
preserve->tdata = abfd->tdata.any;
|
preserve->tdata = abfd->tdata.any;
|
||||||
preserve->arch_info = abfd->arch_info;
|
preserve->arch_info = abfd->arch_info;
|
||||||
|
@ -130,6 +132,7 @@ bfd_preserve_save (bfd *abfd, struct bfd_preserve *preserve)
|
||||||
preserve->section_htab = abfd->section_htab;
|
preserve->section_htab = abfd->section_htab;
|
||||||
preserve->marker = bfd_alloc (abfd, 1);
|
preserve->marker = bfd_alloc (abfd, 1);
|
||||||
preserve->build_id = abfd->build_id;
|
preserve->build_id = abfd->build_id;
|
||||||
|
preserve->cleanup = cleanup;
|
||||||
if (preserve->marker == NULL)
|
if (preserve->marker == NULL)
|
||||||
return FALSE;
|
return FALSE;
|
||||||
|
|
||||||
|
@ -153,7 +156,7 @@ bfd_reinit (bfd *abfd, unsigned int section_id, bfd_cleanup cleanup)
|
||||||
|
|
||||||
/* Restores bfd state saved by bfd_preserve_save. */
|
/* Restores bfd state saved by bfd_preserve_save. */
|
||||||
|
|
||||||
static void
|
static bfd_cleanup
|
||||||
bfd_preserve_restore (bfd *abfd, struct bfd_preserve *preserve)
|
bfd_preserve_restore (bfd *abfd, struct bfd_preserve *preserve)
|
||||||
{
|
{
|
||||||
bfd_hash_table_free (&abfd->section_htab);
|
bfd_hash_table_free (&abfd->section_htab);
|
||||||
|
@ -172,6 +175,7 @@ bfd_preserve_restore (bfd *abfd, struct bfd_preserve *preserve)
|
||||||
its arg, as well as its arg. */
|
its arg, as well as its arg. */
|
||||||
bfd_release (abfd, preserve->marker);
|
bfd_release (abfd, preserve->marker);
|
||||||
preserve->marker = NULL;
|
preserve->marker = NULL;
|
||||||
|
return preserve->cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Called when the bfd state saved by bfd_preserve_save is no longer
|
/* Called when the bfd state saved by bfd_preserve_save is no longer
|
||||||
|
@ -180,6 +184,15 @@ bfd_preserve_restore (bfd *abfd, struct bfd_preserve *preserve)
|
||||||
static void
|
static void
|
||||||
bfd_preserve_finish (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_preserve *preserve)
|
bfd_preserve_finish (bfd *abfd ATTRIBUTE_UNUSED, struct bfd_preserve *preserve)
|
||||||
{
|
{
|
||||||
|
if (preserve->cleanup)
|
||||||
|
{
|
||||||
|
/* Run the cleanup, assuming that all it will need is the
|
||||||
|
tdata at the time the cleanup was returned. */
|
||||||
|
void *tdata = abfd->tdata.any;
|
||||||
|
abfd->tdata.any = preserve->tdata;
|
||||||
|
preserve->cleanup (abfd);
|
||||||
|
abfd->tdata.any = tdata;
|
||||||
|
}
|
||||||
/* It would be nice to be able to free more memory here, eg. old
|
/* It would be nice to be able to free more memory here, eg. old
|
||||||
tdata, but that's not possible since these blocks are sitting
|
tdata, but that's not possible since these blocks are sitting
|
||||||
inside bfd_alloc'd memory. The section hash is on a separate
|
inside bfd_alloc'd memory. The section hash is on a separate
|
||||||
|
@ -252,7 +265,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
|
||||||
save_targ = abfd->xvec;
|
save_targ = abfd->xvec;
|
||||||
|
|
||||||
preserve_match.marker = NULL;
|
preserve_match.marker = NULL;
|
||||||
if (!bfd_preserve_save (abfd, &preserve))
|
if (!bfd_preserve_save (abfd, &preserve, NULL))
|
||||||
goto err_ret;
|
goto err_ret;
|
||||||
|
|
||||||
/* If the target type was explicitly specified, just check that target. */
|
/* If the target type was explicitly specified, just check that target. */
|
||||||
|
@ -381,8 +394,9 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
|
||||||
if (preserve_match.marker == NULL)
|
if (preserve_match.marker == NULL)
|
||||||
{
|
{
|
||||||
match_targ = abfd->xvec;
|
match_targ = abfd->xvec;
|
||||||
if (!bfd_preserve_save (abfd, &preserve_match))
|
if (!bfd_preserve_save (abfd, &preserve_match, cleanup))
|
||||||
goto err_ret;
|
goto err_ret;
|
||||||
|
cleanup = NULL;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -455,7 +469,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
|
||||||
whole bfd and restoring it would be even worse; the first thing
|
whole bfd and restoring it would be even worse; the first thing
|
||||||
you notice is that the cached bfd file position gets out of sync. */
|
you notice is that the cached bfd file position gets out of sync. */
|
||||||
if (preserve_match.marker != NULL)
|
if (preserve_match.marker != NULL)
|
||||||
bfd_preserve_restore (abfd, &preserve_match);
|
cleanup = bfd_preserve_restore (abfd, &preserve_match);
|
||||||
|
|
||||||
if (match_count == 1)
|
if (match_count == 1)
|
||||||
{
|
{
|
||||||
|
|
Loading…
Reference in New Issue