jhutz at cmu.edu
Wed Nov 25 15:58:32 EST 2009
--On Wednesday, November 25, 2009 02:40:56 PM -0500 ghudson at mit.edu wrote:
> Warning: grotty stuff ahead.
> The background: the krb5_get_init_creds system was originally
> specified using a caller-allocated options structure
> (krb5_get_init_creds_opt), which meant options fields couldn't be
> added without breaking the ABI.
> To address this, krb5_gic_opt_ext was created. This is a
> library-allocated structure which is ABI-compatible with
> krb5_get_init_creds_opt (sort of; see below). If you're an API which
> takes an options argument, you call krb5int_gic_opt_to_opte on it,
> with the "force" flag set.
> opt_to_opte has an "interesting" contract: if the input is already an
> extended structure, it creates an alias; otherwise, it creates a copy
> and sets a "shadowed" flag. The caller is supposed to check the
> shadowed flag and free the structure when it is done.
> The problem: this contract is not nestable. If the input of
> opt_to_opte is a copy created by a previous call to opt_to_opte, then
> the output will be an alias which looks like a copy, and that will
> result in a double-free.
> We haven't run into this problem yet because we've never had a reason
> to pass an opt_to_opte'd options structure to an external API. But
> I'm preparing to integrate some IAKERB support code which does;
> specifically, the synchronous wrapper function krb5_get_init_creds
> needs to supply options to krb5_init_creds_init.
> Possible solutions:
> 1. Make opt_to_opte copy the structure if the input is shadowed and
> the force flag is set. This solves the immediate problem and creates
> no extra work if the original caller used the new API.
> 2. Make opt_to_opte copy the structure all the time. This is extra
> work in the common case, but makes the code easier to analyze.
> ("Maybe this is allocated and maybe this is an alias" is not a very
> safe construction.)
> 3. Eliminate the copies. Instead, when peering at new options fields,
> use accessors which can deal with either an extended or non-extended
> options structure. This would touch the most code, but would have the
> same benefits of (2) without the extra allocation-and-copy work. This
> would probably entail changing internal interfaces like
> krb5_get_init_creds to use pointers to krb5_get_init_creds_opt instead
> of krb5_gic_opt_ext.
4. Make opt_to_opte copy the structure only when the input is non-extended,
and add a refcount to the extended version of the structure. Creating an
alias increments the refcount, and callers that currently free when the
"shadowed" flag is set should instead call a put operation that decs the
refcount and frees when appropriate.
More information about the krbdev