linux-kernel - Re: [PATCH] perf scripts python arm-cs-trace-disasm.py: Skip disasm if address continuity is broken

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4dd7f210-c03e-4203-b8e9-1c26a7f8fe79@arm.com>
Date: Wed, 7 Aug 2024 17:48:46 +0100
From: Leo Yan <leo.yan@....com>
To: James Clark <james.clark@...aro.org>,
 Ganapatrao Kulkarni <gankulkarni@...amperecomputing.com>,
 scclevenger@...amperecomputing.com
Cc: acme@...hat.com, coresight@...ts.linaro.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 darren@...amperecomputing.com, james.clark@....com, suzuki.poulose@....com,
 Al.Grant@....com, Mike Leach <mike.leach@...aro.org>
Subject: Re: [PATCH] perf scripts python arm-cs-trace-disasm.py: Skip disasm
 if address continuity is broken

Hi all,

On 8/7/2024 3:53 PM, James Clark wrote:

A minor suggestion: if the discussion is too long, please delete the
irrelevant message ;)

[...]

>> --- a/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> +++ b/tools/perf/scripts/python/arm-cs-trace-disasm.py
>> @@ -257,6 +257,11 @@ def process_event(param_dict):
>>                  print("Stop address 0x%x is out of range [ 0x%x .. 0x%x
>> ] for dso %s" % (stop_addr, int(dso_start), int(dso_end), dso))
>>                  return
>>
>> +       if (stop_addr < start_addr):
>> +               if (options.verbose == True):
>> +                       print("Packet Dropped, Discontinuity detected
>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr,
>> dso))
>> +               return
>> +
> 
> I suppose my only concern with this is that it hides real errors and
> Perf shouldn't be outputting samples that go backwards. Considering that
> fixing this in OpenCSD and Perf has a much wider benefit I think that
> should be the ultimate goal. I'm putting this on my todo list for now
> (including Steve's merging idea).

In the perf's util/cs-etm.c file, it handles DISCONTINUITY with:

   case CS_ETM_DISCONTINUITY:
        /*
         * The trace is discontinuous, if the previous packet is
         * instruction packet, set flag PERF_IP_FLAG_TRACE_END
         * for previous packet.
         */
        if (prev_packet->sample_type == CS_ETM_RANGE)
                prev_packet->flags |= PERF_IP_FLAG_BRANCH |
                                      PERF_IP_FLAG_TRACE_END;

I am wandering if OpenCSD has passed the correct info so Perf decoder can
detect the discontinuity. If yes, then the flag 'PERF_IP_FLAG_TRACE_END' will
be set (it is a general flag in branch sample), then we can consider use it in
the python script to handle discontinuous data.

> 
> But in the mean time what about having a force option?
> 
>> +       if (stop_addr < start_addr):
>> +               if (options.verbose == True or not options.force):
>> +                       print("Packet Dropped, Discontinuity detected
>> [stop_add:0x%x start_addr:0x%x ] for dso %s" % (stop_addr, start_addr,
>> dso))
>> +               if (not options.force):
>> +                       return

If the stop address is less than the start address, it must be something
wrong. In this case, we can report a warning for discontinuity and directly
return (also need to save the `addr` into global variable for next parsing).

I prefer to not add force option for this case - eventually, this will consume
much time for reporting this kind of failure and need to root causing it. A
better way is we just print out the reasoning in the log and continue to dump.

Thanks,
Leo