Re: [PATCH v10 24/24] [RFC] tools/scmi: Add SCMI Telemetry testing tool

[Fayssal Benmlih <Fayssal.Benmlih@arm.com>]

Hi Cristian,

The main test-tool issues reported on V7 still appear present in V10.

open_session() allocates struct tlm_state using malloc() without
initializing it. Optional members such as grps and shmtis can therefore
contain arbitrary pointers when the corresponding resource count is zero.
Please use calloc() or initialize the complete object.

Zero-resource handling is still incomplete. gather_tlm_state() always calls
enumerate_intervals() for the instance, and enumerate_groups() calls it even
when a group reports zero intervals. Display and SHMTI command paths also
dereference optional pointers without consistently checking both the count
and pointer.

Please avoid zero-sized enumerations and make every consumer tolerate a NULL
optional resource.

dump_shmti() and shmti_check() still map only shmti->len bytes before adding
shmti->offset. The mapping needs to cover PAGE_ALIGN(offset + len), and its
original base and complete length must be retained for munmap().

The write loop also still adds write() directly to the cumulative byte
count:

        bytes += write(...);

A -1 result after an earlier successful write can leave bytes nonnegative,
and a zero result can loop indefinitely. Please store the result separately,
handle values less than or equal to zero, and unmap on every exit.

batch_read() allocates space for st->info.num_des entries and then replaces
batch->num_items with args->cnt before writing that many samples. More
command-line IDs than platform DEs therefore write past the allocation.
Please allocate using the validated argument count or reject excessive
arguments before populating the array.

batch_buffer_alloc() still uses unchecked multiplications. If status
allocation fails, the previously allocated item array and wrapper are also
leaked. Please use checked size arithmetic and one unwind path.

More generally, the tool still has no complete session cleanup. The main
device fd, returned SHMTI fds, mappings, interval arrays, DE/group
descriptors, batch buffers and nested allocations remain open or allocated
on normal and error exits.

Please add a session cleanup function and route all exits through it. Apart
from fixing the leaks, that will provide a useful executable example of the
intended UAPI lifetime rules.

Thanks,
Fayçal