UCS/VFS: stale/freed node access when refresh_cb removes the current node or parent

[Zacky-ran]

Describe the bug

ucs_vfs_path_get_info() can access a stale/freed VFS node when the node's refresh_cb removes the currently refreshed object or its parent.

The issue is reproducible in a single thread and does not require RDMA, UCX memory hooks, tcmalloc, GPU, or an external VFS FUSE daemon.

The suspected root cause is that ucs_vfs_node_t::refcount currently mixes two different lifetimes:

1. Logical VFS namespace ownership:

   • object hash
   • path hash
   • parent children list
   • target links list for symlinks

2. Temporary reader memory pin:

   • ucs_vfs_path_get_info()
   • ucs_vfs_path_list_dir()
   • ucs_vfs_path_read_file()

A path reader increments the node refcount before refresh. If the refresh callback removes the node or its parent, the reader ref can prevent the node from being removed at the first removal point, while the node is no longer logically stable. A later removal or the path reader itself can then operate on a stale node, causing assertion failure, refcount underflow, list corruption, or use-after-free-like behavior.

The problematic code path is around:

src/ucs/vfs/base/vfs_obj.c

ucs_vfs_path_get_info()
  -> ucs_vfs_node_increase_refcount(node)
  -> ucs_vfs_refresh_dir(node)
       -> unlock VFS lock
       -> refresh_cb(obj)
            -> ucs_vfs_obj_remove(parent)
            -> ucs_vfs_obj_remove(child)
       -> lock VFS lock again
       -> node->flags &= ~UCS_VFS_FLAGS_DIRTY
  -> continue using node
  -> ucs_vfs_node_decrease_refcount(node)

If refresh_cb() removed the node, ucs_vfs_refresh_dir() and the caller may continue touching a node which is logically removed or already freed.

Steps to Reproduce

UCX environment variables used

No special UCX runtime environment variables are required.

Optional:

UCX_WARN_UNUSED_ENV_VARS=n

Minimal reproducer

The important sequence is:

1. Add /repro/parent
2. Add /repro/parent/child
3. Mark child dirty and set a refresh callback
4. Call ucs_vfs_path_get_info("/repro/parent/child")
5. Inside child's refresh callback:
   a. ucs_vfs_obj_remove(parent)
   b. ucs_vfs_obj_remove(child)

Reproducer code sketch:

#include <ucs/vfs/base/vfs_obj.h>
#include <ucs/status.h>
#include <stdio.h>

static char parent_obj;
static char child_obj;

static void refresh_child(void *obj)
{
    printf("refresh_child: remove parent, then child\n");

    /*
     * First remove the parent. This recursively touches the child while
     * path_get_info() is holding a temporary reader ref on the child.
     */
    ucs_vfs_obj_remove(&parent_obj);

    /*
     * Then remove the same child object explicitly.
     */
    ucs_vfs_obj_remove(&child_obj);
}

int main()
{
    ucs_vfs_path_info_t info;
    ucs_status_t status;

    status = ucs_vfs_obj_add_dir(NULL, &parent_obj, "repro/parent");
    if (status != UCS_OK) {
        printf("add parent failed: %s\n", ucs_status_string(status));
        return 1;
    }

    status = ucs_vfs_obj_add_dir(&parent_obj, &child_obj, "child");
    if (status != UCS_OK) {
        printf("add child failed: %s\n", ucs_status_string(status));
        return 1;
    }

    ucs_vfs_obj_set_dirty(&child_obj, refresh_child);

    printf("calling ucs_vfs_path_get_info(/repro/parent/child)\n");
    status = ucs_vfs_path_get_info("/repro/parent/child", &info);

    printf("ucs_vfs_path_get_info returned: %s\n", ucs_status_string(status));
    return 0;
}

Observed result:

calling ucs_vfs_path_get_info(/repro/parent/child)
refresh_child: remove parent, then child
vfs_obj.c:360 Assertion `khiter != kh_end(&ucs_vfs_obj_context.obj_hash)' failed

Exact line numbers may differ between commits, but the failure is in VFS node removal/hash/list handling after the node has been removed during refresh.

Setup and versions

This reproducer is not hardware dependent.

Tested setup:

OS: Rocky Linux 9.1
CPU architecture: x86_64
Kernel: 5.14.0-162.6.1.el9_1.x86_64

No RDMA/IB/RoCE hardware is required.

No GPU/CUDA is involved.

Additional information

Expected behavior

ucs_vfs_path_get_info() should tolerate the object being removed during refresh_cb().

Acceptable outcomes would be, for example:

UCS_ERR_NO_ELEM

or another safe error after refresh, but it should not assert, underflow refcount, corrupt VFS lists, or continue using a stale node.

Actual behavior

ucs_vfs_path_get_info() can continue using the same ucs_vfs_node_t *node after refresh_cb() removed the object or its parent.

One possible event sequence:

child initial refcount = 1

ucs_vfs_path_get_info("/repro/parent/child")
  child refcount: 1 -> 2

ucs_vfs_refresh_dir(child)
  unlock VFS lock
  refresh_cb(child)

refresh_cb(child)
  ucs_vfs_obj_remove(parent)
    recursively touches child
    child refcount: 2 -> 1
    child is not fully removed because the path reader still holds a ref

  ucs_vfs_obj_remove(child)
    child refcount: 1 -> 0
    child is removed/freed

return from refresh_cb()

ucs_vfs_refresh_dir(child)
  lock VFS lock
  continues touching child node, for example:
    node->flags &= ~UCS_VFS_FLAGS_DIRTY

ucs_vfs_path_get_info()
  continues touching child node
  later calls ucs_vfs_node_decrease_refcount(child)

Depending on memory reuse and exact timing, the failure may appear as:

assertion failure while deleting from obj_hash/path_hash
refcount underflow
corrupted children/links list
stale node access after refresh_cb

Field evidence

This issue was originally noticed while investigating application startup crashes involving UCX VFS node/list corruption.

Several cores had patterns consistent with stale VFS node lifetime handling, for example:

ucs_vfs_node_remove_children()
ucs_vfs_node_decrease_refcount()
ucs_vfs_obj_remove()
uct_md_close()
ucp_init()

In one core, a VFS node had an apparent negative refcount.

In another core, ucs_vfs_obj_add_sym_link(..., "memory_domain") crashed while operating on a corrupted links list. A list pointer had an obviously invalid small value, suggesting stale/freed VFS node memory had been reused by another object.

These field cores are not required to reproduce the issue. The single-thread reproducer above is enough to trigger the lifecycle problem.

Suggested fix direction

I do not think the right fix is to simply hold the global VFS lock across refresh_cb(), because refresh callbacks may legitimately be reentrant and may modify VFS state.

A safer approach may be to separate logical removal from memory reclamation:

1. Add a removed/tombstone state to ucs_vfs_node_t.
2. On first logical remove:
   • mark the node as removed
   • unlink it from obj_hash
   • unlink it from path_hash
   • unlink it from parent children
   • unlink symlink nodes from target links
   • recursively logically remove children exactly once
3. Path-reader refs should only pin memory.
4. Actual ucs_free(node) should be deferred until reader pins drain.
5. After refresh_cb() returns, path reader APIs should re-check whether the node was removed before touching fields such as:
   • flags
   • children
   • path
   • links
   • object hash membership

Conceptually:

ucs_vfs_node_get(node);      /* reader memory pin */
ucs_vfs_node_put(node);      /* release reader memory pin */
ucs_vfs_node_remove(node);   /* logical unlink/tombstone */

The key invariant would be:

A reader pin may keep node memory valid,
but it must not keep the node logically present in the VFS namespace.