linux-kernel - Re: [PATCH 03/16] dyndbg: refine debug verbosity; 1 is basic, 2 more chatty

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]

Date:   Wed, 10 Jun 2020 12:16:15 +0100
From:   Daniel Thompson <daniel.thompson@...aro.org>
To:     jim.cromie@...il.com
Cc:     Jason Baron <jbaron@...mai.com>,
        LKML <linux-kernel@...r.kernel.org>, akpm@...uxfoundation.org,
        Greg KH <gregkh@...uxfoundation.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH 03/16] dyndbg: refine debug verbosity; 1 is basic, 2 more
 chatty

On Tue, Jun 09, 2020 at 01:59:41PM -0600, jim.cromie@...il.com wrote:
> On Mon, Jun 8, 2020 at 5:21 AM Daniel Thompson
> <daniel.thompson@...aro.org> wrote:
> >
> > On Fri, Jun 05, 2020 at 10:26:32AM -0600, Jim Cromie wrote:
> > > The verbose/debug logging done for `cat $MNT/dynamic_debug/control` is
> > > voluminous (2 per control file entry + 2 per PAGE).  Moreover, it just
> > > prints pointer and sequence, which is not useful to a dyndbg user.
> > > So just drop them.
> >
> > I'd assumed these messages where to help the dyndbg implementer rather
> > than the dyndbg user.
> 
> So I thought I was guilty of adding those noisy pr_info()s in the
> ddebug_proc_* functions,
> but I have touched them, changing them to vpr_info().
> In any case, I dont think theyre useful to the implementer either.
> 
> If the verbose messages really are useful to help
> > users who (mis)configure .../control then should the enable/disable
> > control be shadowed in debugfs to make it easy to find?
> >
> 
> I would hesitate to change the API, even if this is just an add-on,
> without changes to existing.
> OTOH, I could see it added as /proc/dynamic_debug/verbose

/proc ? 

I was assuming that if the verbose output of dynamic debug is useful to
the person trying to *use* dynamic_debug then it should be in
/sys/kernel/debug/dynamic_debug/verbose .

If they are only likely useful to the person trying to *implement*
dynamic_debug itself (or to check that the infrastructure is not broken)
then there is no reason to add them to debugfs.


> with this patch, verbose=1 is better focused on showing the parsing process,
> to give user more context as to what his query-command is doing
> verbose=2 additionally shows callsites that match the query, including
> any unchanged (iirc)

I'm still a little confused by what benefit having two levels of
verbosity really is. Why does a user need to turn on verbose mode to
figure out what is happening? Why isn't reading back
.../dynamic_debug/control (perhaps using grep and friends) sufficient?


Daniel.