Re: [PATCH v2 1/3] perf hisi-ptt: Fix PTT trace TLP header parsing

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Medium] The patch fixes endianness and strict-aliasing for packet decoding by replacing unaligned casts with `get_unaligned_le32()`, but misses `hisi_ptt_check_packet_type()` which dictates the packet format for the entire trace.
- [Medium] Missing length validation before packet type detection can lead to an out-of-bounds read and potential crash when processing an auxtrace event with a size less than 4 bytes.
--

--- Patch [1]: [PATCH v2 1/3] perf hisi-ptt: Fix PTT trace TLP header parsing ---
commit 94e8b50cd3127d5e0f6a440877771f9ffe7c6e0e
Author: Sizhe Liu <[email protected]>

perf hisi-ptt: Fix PTT trace TLP header parsing

This commit fixes incorrect field decoding of HiSilicon PTT trace TLP headers
in 4DW format. It replaces the C bitfield struct with explicit bitmasks and
uses get_unaligned_le32() to extract the fields, resolving endianness and
strict-aliasing issues.

> diff --git a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> index a17c423a526dd..5db44556fc4f5 100644
> --- a/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
> +++ b/tools/perf/util/hisi-ptt-decoder/hisi-ptt-pkt-decoder.c
[ ... ]
> @@ -73,29 +75,20 @@ static const char * const hisi_ptt_4dw_pkt_field_name[] = {
[ ... ]
>  static void hisi_ptt_print_pkt(const unsigned char *buf, int pos, const char *desc)
>  {
>  	const char *color = PERF_COLOR_BLUE;
> +	uint8_t byte;
> +	uint32_t dw;
>  	int i;
>  
> +	dw = get_unaligned_le32(buf + pos);

[Severity: Medium]
This is a pre-existing issue, but while this patch fixes endianness and
strict-aliasing during packet decoding, there appears to be another unaligned
pointer cast in hisi_ptt_check_packet_type() in tools/perf/util/hisi-ptt.c:

tools/perf/util/hisi-ptt.c:hisi_ptt_check_packet_type() {
    ...
    uint32_t head = *(uint32_t *)buf;
    ...
}

Will this cause decoding failures on big-endian hosts because the native
unaligned 32-bit cast interprets the little-endian packet header with
incorrect byte order, causing the packet type bitmask check to misidentify
packets?

[Severity: Medium]
This is also a pre-existing issue, but can missing length validation before
packet type detection lead to an out-of-bounds read?

When processing an auxtrace event with a size less than 4 bytes,
hisi_ptt_dump() in tools/perf/util/hisi-ptt.c unconditionally calls
hisi_ptt_check_packet_type() without verifying len:

tools/perf/util/hisi-ptt.c:hisi_ptt_dump() {
    ...
    type = hisi_ptt_check_packet_type(buf);
    ...
}

This immediately dereferences 4 bytes in hisi_ptt_check_packet_type():

tools/perf/util/hisi-ptt.c:hisi_ptt_check_packet_type() {
    ...
    uint32_t head = *(uint32_t *)buf;
    ...
}

Could this result in a heap out-of-bounds read and potential crash?

>  	printf(".");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1