On krb5_db_alloc()

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

On krb5_db_alloc()

Greg Hudson
In the krb5 project, we try to operate on the assumption that
differently compiled code units might use incompatible memory
allocators.  This situation arises frequently on Windows and rarely on
Unix-like systems.  For most interfaces, accomodating this assumption
means providing a free operation corresponding to every allocation
operation; for example, libkrb5 callers are expected to free the result
of krb5_cc_full_name() with krb5_free_string() instead of just free().

For the KDB layer, there are three layers which might be differently
compiled: the caller, the krb5 libraries (libkdb5/libkrb5), and the KDB
module.  Accomodating different allocators is less trivial than for most
interfaces; it is common for callers of libkdb5 to fetch a principal
entry, modify it (such as by changing the key data or tl-data), put the
principal entry, and then free it.

Our current design tries to accomodate KDB modules with different
allocators by including alloc() and free() methods in the KDB interface,
typically implemented by just calling realloc() and free() inside the
module.  When a libkdb5 caller modifies a principal entry, it is
expected to allocate new memory for any fields or sub-fields with
krb5_db_alloc() and free any old fields with krb5_db_free().

Experience has shown that it's really hard to get this right.  If a
caller or custom KDB module needs to manipulate the principal name, it
shouldn't use krb5_copy_principal(), krb5_build_principal(), or
krb5_free_principal(), because those use the libkrb5 allocator.  Similar
problems apply to manipulating key data and occasionally tl-data.

When we screw up this discipline (and we have, many times), we don't
notice except by code inspection.  We are usually testing against
in-tree KDB modules which are built in the same way as the libraries,
and we don't support libkdb5 on Windows.

All of the above concerns also apply to policy entries, although policy
entries have a simpler structure.

I would like to change the contract so that principal and policy entry
memory is all allocated with the allocator visible to the krb5
libraries, not the one visible to the KDB module.  I can see two ways to
accomplish this:

1. Whenever we fetch a principal or policy entry, immediately copy it
into library-allocated memory and then ask the module to free its copy.

2. Document that KDB modules must allocate memory using krb5 library
functions or a compatible allocator.  Modules could call krb5_db_alloc()
to allocate an individual memory region, or a function like
krb5_copy_principal() for a more complicated field.  KDB modules can
also get away with just using malloc() as long as this stuff isn't used
on Windows.

In both of these proposals, krb5_db_alloc() and krb_db_free() would
change to just call realloc() or free() themselves.  The kdb_vftabl
entries for those functions would be removed.

Comments?
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: On krb5_db_alloc()

Sarah Day
On Thursday, May 12, 2016 02:27:50 PM Greg Hudson wrote:

> In the krb5 project, we try to operate on the assumption that
> differently compiled code units might use incompatible memory
> allocators.  This situation arises frequently on Windows and rarely on
> Unix-like systems.  For most interfaces, accomodating this assumption
> means providing a free operation corresponding to every allocation
> operation; for example, libkrb5 callers are expected to free the result
> of krb5_cc_full_name() with krb5_free_string() instead of just free().
>
> For the KDB layer, there are three layers which might be differently
> compiled: the caller, the krb5 libraries (libkdb5/libkrb5), and the KDB
> module.  Accomodating different allocators is less trivial than for most
> interfaces; it is common for callers of libkdb5 to fetch a principal
> entry, modify it (such as by changing the key data or tl-data), put the
> principal entry, and then free it.
>
> Our current design tries to accomodate KDB modules with different
> allocators by including alloc() and free() methods in the KDB interface,
> typically implemented by just calling realloc() and free() inside the
> module.  When a libkdb5 caller modifies a principal entry, it is
> expected to allocate new memory for any fields or sub-fields with
> krb5_db_alloc() and free any old fields with krb5_db_free().
>
> Experience has shown that it's really hard to get this right.  If a
> caller or custom KDB module needs to manipulate the principal name, it
> shouldn't use krb5_copy_principal(), krb5_build_principal(), or
> krb5_free_principal(), because those use the libkrb5 allocator.  Similar
> problems apply to manipulating key data and occasionally tl-data.
>
> When we screw up this discipline (and we have, many times), we don't
> notice except by code inspection.  We are usually testing against
> in-tree KDB modules which are built in the same way as the libraries,
> and we don't support libkdb5 on Windows.
>
> All of the above concerns also apply to policy entries, although policy
> entries have a simpler structure.
>
> I would like to change the contract so that principal and policy entry
> memory is all allocated with the allocator visible to the krb5
> libraries, not the one visible to the KDB module.  I can see two ways to
> accomplish this:
>
> 1. Whenever we fetch a principal or policy entry, immediately copy it
> into library-allocated memory and then ask the module to free its copy.
>
> 2. Document that KDB modules must allocate memory using krb5 library
> functions or a compatible allocator.  Modules could call krb5_db_alloc()
> to allocate an individual memory region, or a function like
> krb5_copy_principal() for a more complicated field.  KDB modules can
> also get away with just using malloc() as long as this stuff isn't used
> on Windows.
>
> In both of these proposals, krb5_db_alloc() and krb_db_free() would
> change to just call realloc() or free() themselves.  The kdb_vftabl
> entries for those functions would be removed.
>
> Comments?
> _______________________________________________
> krbdev mailing list             [hidden email]
> https://mailman.mit.edu/mailman/listinfo/krbdev

I'm all for reducing complexity, and your method sounds perfectly fine to me.

--
Sarah Day
Identity & Access Management
MIT | IS&T | Platform & Systems Integration
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: On krb5_db_alloc()

Simo Sorce-3
In reply to this post by Greg Hudson
On Thu, 2016-05-12 at 14:27 -0400, Greg Hudson wrote:

> In the krb5 project, we try to operate on the assumption that
> differently compiled code units might use incompatible memory
> allocators.  This situation arises frequently on Windows and rarely on
> Unix-like systems.  For most interfaces, accomodating this assumption
> means providing a free operation corresponding to every allocation
> operation; for example, libkrb5 callers are expected to free the result
> of krb5_cc_full_name() with krb5_free_string() instead of just free().
>
> For the KDB layer, there are three layers which might be differently
> compiled: the caller, the krb5 libraries (libkdb5/libkrb5), and the KDB
> module.  Accomodating different allocators is less trivial than for most
> interfaces; it is common for callers of libkdb5 to fetch a principal
> entry, modify it (such as by changing the key data or tl-data), put the
> principal entry, and then free it.
>
> Our current design tries to accomodate KDB modules with different
> allocators by including alloc() and free() methods in the KDB interface,
> typically implemented by just calling realloc() and free() inside the
> module.  When a libkdb5 caller modifies a principal entry, it is
> expected to allocate new memory for any fields or sub-fields with
> krb5_db_alloc() and free any old fields with krb5_db_free().
>
> Experience has shown that it's really hard to get this right.  If a
> caller or custom KDB module needs to manipulate the principal name, it
> shouldn't use krb5_copy_principal(), krb5_build_principal(), or
> krb5_free_principal(), because those use the libkrb5 allocator.  Similar
> problems apply to manipulating key data and occasionally tl-data.
>
> When we screw up this discipline (and we have, many times), we don't
> notice except by code inspection.  We are usually testing against
> in-tree KDB modules which are built in the same way as the libraries,
> and we don't support libkdb5 on Windows.
>
> All of the above concerns also apply to policy entries, although policy
> entries have a simpler structure.
>
> I would like to change the contract so that principal and policy entry
> memory is all allocated with the allocator visible to the krb5
> libraries, not the one visible to the KDB module.  I can see two ways to
> accomplish this:
>
> 1. Whenever we fetch a principal or policy entry, immediately copy it
> into library-allocated memory and then ask the module to free its copy.

This is wasteful.

> 2. Document that KDB modules must allocate memory using krb5 library
> functions or a compatible allocator.  Modules could call krb5_db_alloc()
> to allocate an individual memory region, or a function like
> krb5_copy_principal() for a more complicated field.  KDB modules can
> also get away with just using malloc() as long as this stuff isn't used
> on Windows.
>
> In both of these proposals, krb5_db_alloc() and krb_db_free() would
> change to just call realloc() or free() themselves.  The kdb_vftabl
> entries for those functions would be removed.
>
> Comments?

I'm for 2.

Simo.

--
Simo Sorce * Red Hat, Inc * New York

_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev
Reply | Threaded
Open this post in threaded view
|

Re: On krb5_db_alloc()

Will Fiveash-2
On Mon, May 16, 2016 at 09:47:45AM -0400, Simo Sorce wrote:

> On Thu, 2016-05-12 at 14:27 -0400, Greg Hudson wrote:
> > I would like to change the contract so that principal and policy entry
> > memory is all allocated with the allocator visible to the krb5
> > libraries, not the one visible to the KDB module.  I can see two ways to
> > accomplish this:
> >
> > 1. Whenever we fetch a principal or policy entry, immediately copy it
> > into library-allocated memory and then ask the module to free its copy.
>
> This is wasteful.
>
> > 2. Document that KDB modules must allocate memory using krb5 library
> > functions or a compatible allocator.  Modules could call krb5_db_alloc()
> > to allocate an individual memory region, or a function like
> > krb5_copy_principal() for a more complicated field.  KDB modules can
> > also get away with just using malloc() as long as this stuff isn't used
> > on Windows.
> >
> > In both of these proposals, krb5_db_alloc() and krb_db_free() would
> > change to just call realloc() or free() themselves.  The kdb_vftabl
> > entries for those functions would be removed.
> >
> > Comments?
>
> I'm for 2.

I agree with Simo.

--
Will Fiveash
Oracle Solaris Software Engineer
_______________________________________________
krbdev mailing list             [hidden email]
https://mailman.mit.edu/mailman/listinfo/krbdev