Hi Aaron,
On Thu, Mar 27, 2025 at 12:07:18AM -0400, Aaron Merey wrote:
> Ensure that dwarf_lock is held before accessing next_tu_offset and
> next_cu_offset.
>
> This fixes a TOCTOU bug in __libdw_findcu that causes NULL to be
> incorrectly returned.
Could you explain what the issue is in a few more words?
I am not sure I fully follow what the lock protects here precisely.
The patch and the mention of TOCTOU seem to indicate it is about this in
particular:
struct Dwarf_CU **found = eu_tfind (&fake, tree, findcu_cb);
if (found != NULL)
return *found;
Does that mean that despite eu_tfind internally taking a lock on the
tree lock, the returned result can still change after the call? If
that is the case how exactly does that happen?
Or isn't this the part that needs guarding?
Asking because if it is this eu_tfind check return != NULL construct
then there are more places that might need extra guarding (or we have
to write better eu-search wrappers?)
Thanks,
Mark
> Signed-off-by: Aaron Merey <ame...@redhat.com>
> ---
> libdw/libdw_findcu.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/libdw/libdw_findcu.c b/libdw/libdw_findcu.c
> index 8805af9b..0e4dcc37 100644
> --- a/libdw/libdw_findcu.c
> +++ b/libdw/libdw_findcu.c
> @@ -240,6 +240,8 @@ struct Dwarf_CU *
> internal_function
> __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool v4_debug_types)
> {
> + mutex_lock (dbg->dwarf_lock);
> +
> search_tree *tree = v4_debug_types ? &dbg->tu_tree : &dbg->cu_tree;
> Dwarf_Off *next_offset
> = v4_debug_types ? &dbg->next_tu_offset : &dbg->next_cu_offset;
> @@ -249,9 +251,10 @@ __libdw_findcu (Dwarf *dbg, Dwarf_Off start, bool
> v4_debug_types)
> struct Dwarf_CU **found = eu_tfind (&fake, tree, findcu_cb);
> struct Dwarf_CU *result = NULL;
> if (found != NULL)
> - return *found;
> -
> - mutex_lock (dbg->dwarf_lock);
> + {
> + mutex_unlock (dbg->dwarf_lock);
> + return *found;
> + }
>
> if (start < *next_offset)
> __libdw_seterrno (DWARF_E_INVALID_DWARF);
> --
> 2.49.0
>
