Skip Menu |
 

Subject: memory ccache cursors are invalidated by initialize
Memory ccache objects contain a linked list of credentials. The
iteration cursor is a pointer to one of the list elements. If the
cache is initialized by one thread while another thread is iterating
over it, the second thread's cursor contains a dangling pointer and the
process will likely crash.

(This behavior can also be demonstrated in a single-threaded caller,
but the caller would have to be doing something obtuse.)
Download (untitled) / with headers
text/plain 1.7KiB
A closely related issue is that all memory ccache handles for the
same name share data pointers, and destroying a memory ccache
invalidates the data pointer for all of the shared handles, as well
as any iterators produced from those handles. So the following code
crashes, with or without multiple threads:

krb5_cc_resolve(context, "MEMORY:x", &cc1);
krb5_cc_resolve(context, "MEMORY:x", &cc2);
krb5_cc_destroy(context, cc1);
krb5_cc_destroy(context, cc2);

(Any other access to cc2 would also crash except for
krb5_cc_close().)

We could solve this second issue with a reference count on the data
object--one increment for the table slot and one for each active
handle. krb5_mcc_destroy() would just remove the increment for the
table slot and then close the handle; krb5_mcc_close() would remove
the reference count for the handle and destroy the data object if the
counts hits zero. Since cursors are subsidiary to ccache handles,
when the reference count drops to 0 we can be confident that there
are no active cursors for that data object.

This fix doesn't solve the original issue. For the original issue we
could introduce a generation counter, expanding each cursor into a
(generation, pointer) pair. krb5_cc_initialize() increments the
generation counter, and krb5_mcc_next_cred() checks the generation
counter before accessing the pointer.

This solution isn't totally satisfactory because it still leaves
invalid pointers lying around, even if they won't be accessed. A
runtime analyzer (not one I'm aware of, but perhaps in the future)
might flag that as a potential bug. We could convert the linked list
to an array and store an array offset in cursors to address that, or
just ignore it.
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.3KiB

Fix bugs with concurrent use of MEMORY ccaches

A memory ccache iterator stores an alias into the cache object's
linked list of credentials. If the cache is reinitialized while the
iterator is active, the alias becomes invalid. Also, multiple handles
referencing the same memory ccache all use aliases to the same data
object; if one of the handles is destroyed, the other contains a
dangling pointer.

Fix the first issue by adding a generation counter to the cache and to
cursors, incremented each time the cache is initialized or destroyed.
Check the generation on each cursor step and end the iteration if the
list was invalidated. Fix the second issue by adding a reference
count to the cache object, counting one reference for the table slot
and one for each open handle. Empty the cache object on each destroy
operation, but only release the object when the last handle to it is
destroyed or closed.

Add regression tests for the two issues to t_cc.c.

The first issue was reported by Sorin Manolache.

https://github.com/krb5/krb5/commit/146dadec8fe7ccc4149eb2e3f577cc320aee6efb
Author: Greg Hudson <ghudson@mit.edu>
Commit: 146dadec8fe7ccc4149eb2e3f577cc320aee6efb
Branch: master
src/lib/krb5/ccache/cc_memory.c | 164 ++++++++++++++++++++++++--------------
src/lib/krb5/ccache/t_cc.c | 51 ++++++++++++
2 files changed, 154 insertions(+), 61 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.4KiB

Fix bugs with concurrent use of MEMORY ccaches

A memory ccache iterator stores an alias into the cache object's
linked list of credentials. If the cache is reinitialized while the
iterator is active, the alias becomes invalid. Also, multiple handles
referencing the same memory ccache all use aliases to the same data
object; if one of the handles is destroyed, the other contains a
dangling pointer.

Fix the first issue by adding a generation counter to the cache and to
cursors, incremented each time the cache is initialized or destroyed.
Check the generation on each cursor step and end the iteration if the
list was invalidated. Fix the second issue by adding a reference
count to the cache object, counting one reference for the table slot
and one for each open handle. Empty the cache object on each destroy
operation, but only release the object when the last handle to it is
destroyed or closed.

Add regression tests for the two issues to t_cc.c.

The first issue was reported by Sorin Manolache.

(cherry picked from commit 146dadec8fe7ccc4149eb2e3f577cc320aee6efb)

https://github.com/krb5/krb5/commit/6d784809fe07c2d5f60c1a692bcac08b0d40f0a7
Author: Greg Hudson <ghudson@mit.edu>
Commit: 6d784809fe07c2d5f60c1a692bcac08b0d40f0a7
Branch: krb5-1.16
src/lib/krb5/ccache/cc_memory.c | 164 ++++++++++++++++++++++++--------------
src/lib/krb5/ccache/t_cc.c | 51 ++++++++++++
2 files changed, 154 insertions(+), 61 deletions(-)
From: ghudson@mit.edu
Subject: git commit
Download (untitled) / with headers
text/plain 1.4KiB

Fix bugs with concurrent use of MEMORY ccaches

A memory ccache iterator stores an alias into the cache object's
linked list of credentials. If the cache is reinitialized while the
iterator is active, the alias becomes invalid. Also, multiple handles
referencing the same memory ccache all use aliases to the same data
object; if one of the handles is destroyed, the other contains a
dangling pointer.

Fix the first issue by adding a generation counter to the cache and to
cursors, incremented each time the cache is initialized or destroyed.
Check the generation on each cursor step and end the iteration if the
list was invalidated. Fix the second issue by adding a reference
count to the cache object, counting one reference for the table slot
and one for each open handle. Empty the cache object on each destroy
operation, but only release the object when the last handle to it is
destroyed or closed.

Add regression tests for the two issues to t_cc.c.

The first issue was reported by Sorin Manolache.

(cherry picked from commit 146dadec8fe7ccc4149eb2e3f577cc320aee6efb)

https://github.com/krb5/krb5/commit/9c70a33b30919c217167b5e2191c871229e5abbf
Author: Greg Hudson <ghudson@mit.edu>
Commit: 9c70a33b30919c217167b5e2191c871229e5abbf
Branch: krb5-1.15
src/lib/krb5/ccache/cc_memory.c | 164 ++++++++++++++++++++++++--------------
src/lib/krb5/ccache/t_cc.c | 51 ++++++++++++
2 files changed, 154 insertions(+), 61 deletions(-)