[PATCH 2.40 v2 3/3] libctf: ctf-link outdated input check faulty
Nick Alcock
nick.alcock@oracle.com
Wed Jan 11 17:21:15 GMT 2023
Is this patch alone OK for 2.40? (After I push this series to master.
Tomorrow, not today... all tests passed, crash on i686-pc-linux-gnu
fixed, valgrind-clean, asan-clean.)
(I plan to backport it all the way back to 2.36.)
On 10 Jan 2023, Nick Alcock via Binutils spake thusly:
> This check has a pair of faults which, combined, can lead to memory
> corruption. Firstly, it assumes that the values of the ctf_link_inputs
> hash are ctf_dict_t's: they are not, they are ctf_link_input_t's, a much
> shorter structure. So the flags check which is the core of this is
> faulty (but happens, by chance, to give the right output on most
> architectures, since usually we happen to get a 0 here, so the test that
> checks this usually passes). Worse, the warning that is emitted when
> the test fails is added to the wrong dict -- it's added to the input
> dict, whose warning list is never consumed, rendering the whole check
> useless. But the dict it adds to is still the wrong type, so we end up
> overwriting something deep in memory (or, much more likely,
> dereferencing a garbage pointer and crashing).
>
> Fixing both reveals another problem: the link input is an *archive*
> consisting of multiple members, so we have to consider whether to check
> all of them for the outdated-func-info thing we are checking here.
> However, no compiler exists that emits a mixture of members with this
> flag on and members with it off, and the linker always reserializes (and
> upgrades) such things when it sees them: so all members in a given
> archive must have the same value of the flag, so we only need to check
> one member per input archive.
>
> libctf/
> PR libctf/29983
> * ctf-link.c (ctf_link_warn_outdated_inputs): Get the types of
> members of ctf_link_inputs right, fixing a possible spurious
> tesst failure / wild pointer deref / overwrite. Emit the
> warning message into the right dict.
> ---
> libctf/ctf-link.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/libctf/ctf-link.c b/libctf/ctf-link.c
> index 2837168b2a6..df8fa3b9d9b 100644
> --- a/libctf/ctf-link.c
> +++ b/libctf/ctf-link.c
> @@ -1848,19 +1848,42 @@ ctf_link_warn_outdated_inputs (ctf_dict_t *fp)
> {
> ctf_next_t *i = NULL;
> void *name_;
> - void *ifp_;
> + void *input_;
> int err;
>
> - while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &ifp_)) == 0)
> + while ((err = ctf_dynhash_next (fp->ctf_link_inputs, &i, &name_, &input_)) == 0)
> {
> const char *name = (const char *) name_;
> - ctf_dict_t *ifp = (ctf_dict_t *) ifp_;
> + ctf_link_input_t *input = (ctf_link_input_t *) input_;
> + ctf_next_t *j = NULL;
> + ctf_dict_t *ifp;
> + int err;
> +
> + /* We only care about CTF archives by this point: lazy-opened archives
> + have always been opened by this point, and short-circuited entries have
> + a matching corresponding archive member. Entries with NULL clin_arc can
> + exist, and constitute old entries renamed via a name changer: the
> + renamed entries exist elsewhere in the list, so we can just skip
> + those. */
> +
> + if (!input->clin_arc)
> + continue;
> +
> + /* All entries in the archive will necessarily contain the same
> + CTF_F_NEWFUNCINFO flag, so we only need to check the first. We don't
> + even need to do that if we can't open it for any reason at all: the
> + link will fail later on regardless, since an input can't be opened. */
> +
> + ifp = ctf_archive_next (input->clin_arc, &j, NULL, 0, &err);
> + if (!ifp)
> + continue;
> + ctf_next_destroy (j);
>
> if (!(ifp->ctf_header->cth_flags & CTF_F_NEWFUNCINFO)
> && (ifp->ctf_header->cth_varoff - ifp->ctf_header->cth_funcoff) > 0)
> - ctf_err_warn (ifp, 1, 0, _("linker input %s has CTF func info but uses "
> - "an old, unreleased func info format: "
> - "this func info section will be dropped."),
> + ctf_err_warn (fp, 1, 0, _("linker input %s has CTF func info but uses "
> + "an old, unreleased func info format: "
> + "this func info section will be dropped."),
> name);
> }
> if (err != ECTF_NEXT_END)
--
NULL && (void)
More information about the Binutils
mailing list