linux-kernel - Re: [PATCH v4 1/2] perf/core: Bail out early if the request AUX area is out of bound

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: <20230906070238.GC388456@leoy-huanghe.lan>
Date:   Wed, 6 Sep 2023 15:02:38 +0800
From:   Leo Yan <leo.yan@...aro.org>
To:     Shuai Xue <xueshuai@...ux.alibaba.com>
Cc:     James Clark <james.clark@....com>,
        Peter Zijlstra <peterz@...radead.org>,
        alexander.shishkin@...ux.intel.com, mingo@...hat.com,
        baolin.wang@...ux.alibaba.com, acme@...nel.org,
        mark.rutland@....com, jolsa@...nel.org, namhyung@...nel.org,
        irogers@...gle.com, adrian.hunter@...el.com,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
        nathan@...nel.org, bpf@...r.kernel.org
Subject: Re: [PATCH v4 1/2] perf/core: Bail out early if the request AUX area
 is out of bound

Hi Shuai,

On Wed, Sep 06, 2023 at 11:27:38AM +0800, Shuai Xue wrote:

[...]

> >>> +	/* Can't allocate more than MAX_ORDER */
> >>
> >> The comment is confused.  I'd like to refine it as:
> >>
> >>   /*
> >>    * kcalloc_node() is unable to allocate buffer if the size is larger
> >>    * than: PAGE_SIZE << MAX_ORDER; directly bail out in this case.
> >>    */
> > 
> > Hi, Leo,
> > 
> > Thank you for your quick feedback. The comment is simplified from Peter's reply in v2
> > version. Your refined comment is more detailed and it makes sense to me, I would like
> > to adopt it if @Peter has no other opinions.
> > 
> >> To be honest, I am not sure if perf core maintainers like this kind
> >> thing or not.  Please seek their opinion before you move forward.
> >>
> > 
> > and hi, all perf core maintainers,
> > 
> > I have not received explicit objection from perf core maintainers @Peter or @James so
> > I moved forward to address their comments. It's fine to me to wait for more opinions from
> > perf core maintainers.
> > 
> > Best Regards,
> > Shuai
> > 
> 
> Hi, Leo, and all folks,
> 
> Any more comments? Should I move forward to send a new patch?

I am afraid I cannot give a reliable suggestion.

Anyway, I personally think the returned error value in this patch is
better than the kernel oops since the kernel oops is a bit scary for
tool's users ;)   Another reason is the perf core layer should report
error earlier rather than relying on the page buddy allocation layer
to detect the memory allocation failure, which is easier for both
developers and users to understand the failure.

IMHO, a good practice is to respin a new patch set and send out for
review.  Good luck!

Thanks,
Leo