linux-kernel - Re: [PATCH v2 2/2] livepatch: Use static buffer for debugging messages under rq lock

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: <20190507131907.ep5g2hi4hp2lvy3d@treble>
Date:   Tue, 7 May 2019 08:19:07 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Jiri Kosina <jikos@...nel.org>, Miroslav Benes <mbenes@...e.cz>,
        Joe Lawrence <joe.lawrence@...hat.com>,
        Kamalesh Babulal <kamalesh@...ux.vnet.ibm.com>,
        live-patching@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] livepatch: Use static buffer for debugging
 messages under rq lock

On Tue, May 07, 2019 at 01:50:29PM +0200, Petr Mladek wrote:
> On Mon 2019-05-06 19:43:19, Josh Poimboeuf wrote:
> > On Tue, Apr 30, 2019 at 11:10:49AM +0200, Petr Mladek wrote:
> > > klp_try_switch_task() is called under klp_mutex. The buffer for
> > > debugging messages might be static.
> > 
> > The patch description is missing a "why" (presumably to reduce stack
> > usage).
> 
> Exactly. I thought that it was obvious. But I am infected by printk
> code where line buffers are 1k and nobody wants them on the stack.
> 
> 128bytes in klp_try_switch_task() context are acceptable but
> it is still rather big buffer.
> 
> OK, what about the following commit message?
> 
> "klp_try_switch_task() is called under klp_mutex. The buffer for
> debugging messages might be static to reduce stack usage."

It's better to use imperative language.  It would also be good to
reverse the order of the wording by starting with the problem.

Something like:

The err_buf array uses 128 bytes of stack space.  Move it off the stack
by making it static.  It's safe to use a shared buffer because
klp_try_switch_task() is called under klp_mutex.

-- 
Josh