linux-kernel - Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

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: <CAJfuBxwA_1kkjeqDRCDKvJ+PesUgYcVwdQeL-ePM7+ZFLfnO0w@mail.gmail.com>
Date:   Tue, 14 Nov 2023 08:40:32 -0700
From:   jim.cromie@...il.com
To:     Łukasz Bartosik <lb@...ihalf.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Jason Baron <jbaron@...mai.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kees Cook <keescook@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        Guenter Roeck <groeck@...gle.com>,
        Yaniv Tzoreff <yanivt@...gle.com>,
        Benson Leung <bleung@...gle.com>,
        Vincent Whitchurch <vincent.whitchurch@...s.com>,
        Pekka Paalanen <ppaalanen@...il.com>,
        Sean Paul <seanpaul@...omium.org>,
        Daniel Vetter <daniel@...ll.ch>, linux-kernel@...r.kernel.org,
        upstream@...ihalf.com
Subject: Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

On Tue, Nov 14, 2023 at 12:45 AM Łukasz Bartosik <lb@...ihalf.com> wrote:
>
> wt., 14 lis 2023 o 02:08 <jim.cromie@...il.com> napisał(a):
> >

> > > > > Also I think we need a reserved keyword for writing debug logs to
> > > > > trace events - maybe "event" keyword ?
> > > >
> > > > do you mean "event" as a selector, like module, function, class, etc ?
> > > > if so, what are the values ?
> > > > any event under  /sys/kernel/debug/tracing/events/ ?
> > > >
> > > > how does this get used ?
> > > >
> > >
> > > I meant that we need to reserve name/keyword to enable writing debug
> > > logs to trace events (prdbg and devdbg), for example
> > > echo module usbcore +T:event > /proc/dynamic_debug/control
> > >
> > > Or do you anticipate other way to do it ?
> >
> > way back, when I had even fewer clues,
> > I sent patches to call trace-printk when +T was set.
> > Steve didnt like it, I think cuz it could flood the tracebuf.
> >
> > Thats why I added the prdbg and devdbg event-types,
> > so that they could be disabled easily using /sys/kernel/debug/tracing/
> > putting them squarely under trace-control.
> >
> > Note that this puts 2 off-switches in series,
> > both tracefs and >control can disable all the pr_debug traffic,
> > tracefs by event-type, and >control at individual callsite level.
> >
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/enable
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable
> >
> > I briefly thought about linking the 2 off-switches,
> > but punted cuz I thought it complicated things,
> > (how exactly would they get coupled?)
> > and I didnt want to distract from larger goals
> >
> > Does that address your question ?
> >
>
> Jim,
>
> Thanks but it doesn't answer my question.
>
> How do you plan to enable output to tracefs event at a callsite level ?
>
> In my original proposal it was enabled by setting trace destination to
> 0. Since we are moving to names instead of numbers now I guess we need
> to reserve a name for it not to clash with trace instance names
> provided by users. That's why I proposed to reserve name "event" for
> that purpose and be used as +T:event.
>

Ok, I got your point now.

how about we call it "0" ?
it should be an obvious "magical" value,
cuz it doesnt need to be open'd, and cant be close'd

then we can revert to global tracebuf by its "name"
echo module foo +T:0 > /proc/dynamic_debug/control

we probably should also limit the trace-instance-names to ^\w+

> Or did I miss answer to that in our long discussion :> ?

nope :-)

thanks,
Jim

>
> Thanks,
> Lukasz
>
> > On a related point, I also added drm_dbg and drm_devdbg.
> > Those are issued from __drm_dbg & __drm_dev_dbg
> >  respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.
> >
> > Im not sure theyre more useful than confusing yet.
> >
> > >
> > > > >
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > > > > the existing setting)
> > > > > > > >
> > > > > > >
> > > > > > > Sounds good.
> > > > > > >
> > > > > >
> > > > > > :-)