linux-kernel - Re: [PATCH] perf script python: Fix buffer size to report iregs in perf script

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] [day] [month] [year] [list]

Date:   Wed, 7 Jul 2021 11:04:49 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     kajoljain <kjain@...ux.ibm.com>
Cc:     Jiri Olsa <jolsa@...hat.com>, maddy@...ux.vnet.ibm.com,
        atrajeev@...ux.vnet.ibm.com, linux-kernel@...r.kernel.org,
        ravi.bangoria@...ux.ibm.com, linux-perf-users@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, rnsastry@...ux.ibm.com,
        "Paul A. Clarke" <pc@...ibm.com>
Subject: Re: [PATCH] perf script python: Fix buffer size to report iregs in
 perf script

Em Wed, Jul 07, 2021 at 11:16:20AM +0530, kajoljain escreveu:
> On 7/7/21 12:45 AM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Jul 06, 2021 at 05:26:12PM +0530, kajoljain escreveu:
> >> On 6/29/21 12:39 PM, kajoljain wrote:
> >>> On 6/28/21 8:19 PM, Paul A. Clarke wrote:
> >>>> On Mon, Jun 28, 2021 at 11:53:41AM +0530, Kajol Jain wrote:
> >>>>> @@ -713,7 +711,16 @@ static void set_regs_in_dict(PyObject *dict,
> >>>>>  			     struct evsel *evsel)
> >>>>>  {
> >>>>>  	struct perf_event_attr *attr = &evsel->core.attr;
> >>>>> -	char bf[512];
> >>>>> +
> >>>>> +	/*
> >>>>> +	 * Here value 28 is a constant size which can be used to print
> >>>>> +	 * one register value and its corresponds to:
> >>>>> +	 * 16 chars is to specify 64 bit register in hexadecimal.
> >>>>> +	 * 2 chars is for appending "0x" to the hexadecimal value and
> >>>>> +	 * 10 chars is for register name.
> >>>>> +	 */
> >>>>> +	int size = __sw_hweight64(attr->sample_regs_intr) * 28;
> >>>>> +	char bf[size];

> >>>> I propose using a template rather than a magic number here. Something like:
> >>>> const char reg_name_tmpl[] = "10 chars  ";
> >>>> const char reg_value_tmpl[] = "0x0123456789abcdef";
> >>>> const int size = __sw_hweight64(attr->sample_regs_intr) +
> >>>>                  sizeof reg_name_tmpl + sizeof reg_value_tmpl;

> >>>    Thanks for reviewing the patch. Yes these are
> >>> some standardization we can do by creating macros for different
> >>> fields.
> >>> The basic idea is, we want to provide significant buffer size
> >>> based on number of registers present in sample_regs_intr to accommodate
> >>> all data.

> >>    Is the approach used in this patch looks fine to you?

> > Yeah, and the comment you provide right above it explains it, so I think
> > that is enough, ok?
 
>     Thanks for reviewing it. As you said added comment already explains
> why we are taking size constant as 28, should we skip adding macros part?
> Can you pull this patch.

Sure.

- Arnaldo