cli: fix memory leak in volume status detail (#4292)

[ThalesBarretto]

Ownership model

cli_volume_status_t has seven pointer fields with two different ownership models:

Owned (heap-allocated, caller must free):

• brick — GF_MALLOC before the loop
• pid_str — gf_asprintf each iteration
• free, total — gf_uint64_2human_readable inside cli_get_detail_status

Borrowed (point into dict internals, released by dict_unref at out:):

• fs_name, mount_options, device, inode_size — all from dict_get_str

The bug

status is declared once outside the loop with = {0}. Each iteration overwrites the owned pointers with new heap allocations. Before this fix, only pid_str was freed inside the loop body and only brick at the out: label. The other two owned fields (free, total) were overwritten each iteration — previous values lost, two strings leaked per brick. The reporter's ASan trace confirms 144 bytes in 4 allocations (2 calls × 2 bricks).

What the fix does

1. free and total freed in the loop — matching what was already done for pid_str.

2. pid_str, free, total freed at out: — covers the two goto out error paths mid-iteration (gf_asprintf failure and cli_get_detail_status failure) where the current iteration's allocations haven't been freed yet.

3. Explicit NULLing after each in-loop GF_FREE — on the normal path the loop finishes, execution falls through cont: to out:, which calls GF_FREE on the same pointers. Without NULLing that's a double-free; with it, GF_FREE(NULL) is a no-op.

Fixes: #4292

[ThalesBarretto]

@sanjurakonde would you mind taking a look here, i think this one is pretty straightforward

[gglluukk]

There is no need for:

1. NULLing: status.pid_str = NULL;
2. FREEinig: GF_FREE(status.free); and GF_FREE(status.total); as they aren't any MALLOCated
3. The only patch according to your AI's logic is needed at line 7597 before goto out;:

           if ((cmd & GF_CLI_STATUS_DETAIL)) {
            ret = cli_get_detail_status(dict, i, &status);
            if (ret) {
                GF_FREE(status.pid_str); // <--
                goto out;
            }

4. The CLI is merely a command-line utility and is not part of the core storage logic, so bugs in it do not necessarily affect the storage engine itself.

Note that generating AI slop wastes electricity and the time of real human developers.

[ThalesBarretto]

There is no need for:

1. NULLing: status.pid_str = NULL;
2. FREEinig: GF_FREE(status.free); and GF_FREE(status.total); as they aren't any MALLOCated
3. The only patch according to your AI's logic is needed at line 7597 before goto out;:

           if ((cmd & GF_CLI_STATUS_DETAIL)) {
            ret = cli_get_detail_status(dict, i, &status);
            if (ret) {
                GF_FREE(status.pid_str); // <--
                goto out;
            }

4. The CLI is merely a command-line utility and is not part of the core storage logic, so bugs in it do not necessarily affect the storage engine itself.

Note that generating AI slop wastes electricity and the time of real human developers.

Thanks for your interest in glusterfs, your contributions are welcome, feel free to propose a patch

[ThalesBarretto]

Thanks for the review. Let me address each point with code references.

On point 2 — status.free and status.total are heap-allocated. The chain is:

• cli_get_detail_status assigns status->free = gf_uint64_2human_readable(free);
• gf_uint64_2human_readable calls gf_asprintf(&str, ...) on every branch and returns the heap pointer

They must be freed by the caller, unlike the four dict_get_str fields (fs_name, mount_options, device, inode_size) which are borrowed from the dict and released by dict_unref.

On point 3 — freeing pid_str only before the goto out at line 7597 covers one error path, but misses the main bug: the per-iteration leak of free and total on the normal (non-error) path. Each successful iteration overwrites those pointers with new gf_asprintf allocations without freeing the previous ones — the old references are lost and the memory becomes unreachable. That is the leaked memory the ASan trace in #4292 reports.

On point 1 — the NULLing fixes a second, separate issue. After GF_FREE(status.pid_str) in the loop body, the pointer still holds the address of freed memory — it dangles. On the normal path, execution falls straight through from the loop end at line 7610 to out: at line 7618 (no return or goto between them). The out: label calls GF_FREE on that same dangling pointer — a double-free. NULLing after each in-loop GF_FREE makes the second free a no-op. Without it, the in-loop frees would fix the leak but introduce a double-free: one memory safety bug traded for another.

On point 4 — agreed, the CLI is short-lived and this is not a critical storage bug. It is a correctness fix for a reported issue (#4292) with an ASan trace.