Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jim Cromie <jim.cromie@gmail.com>
To:     jbaron@akamai.com, linux-kernel@vger.kernel.org,
        akpm@linuxfoundation.org, gregkh@linuxfoundation.org
Cc:     linux@rasmusvillemoes.dk, Jim Cromie <jim.cromie@gmail.com>
Subject: [PATCH v4 15/17] dyndbg: export ddebug_exec_queries
Date:   Sat, 20 Jun 2020 12:06:41 -0600
Message-Id: <20200620180643.887546-16-jim.cromie@gmail.com>
In-Reply-To: <20200620180643.887546-1-jim.cromie@gmail.com>
References: <20200620180643.887546-1-jim.cromie@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Export ddebug_exec_queries() for use by modules.

This will allow module authors to control all their *pr_debug*s
dynamically.  And since ddebug_exec_queries() is what implements
"echo $query >control", it gives the same complete control.

Virtues of this:
- simplicity. just an export.
- full control over any/all subsets of callsites.
- same "query/command-string" in code and console
- full callsite selectivity with module file line format

Format in particular deserves special attention; it is where
low-hanging fruit will be found.

Consider: drivers/gpu/drm/amd/display/include/logger_types.h:

  #define DC_LOG_SURFACE(...)          pr_debug("[SURFACE]:"__VA_ARGS__)
  #define DC_LOG_HW_LINK_TRAINING(...) pr_debug("[HW_LINK_TRAINING]:"__VA_ARGS__)
  .. 9 more ..

Thats 11 string prefixes, used in 804 callsites across the module.
Clearly a systematized classification of those callsites.  And one Id
expect to see repeated often.

Using ddebug_exec_queries(), authors can operate on those
classifications as a unitary set:

    echo format="[SURFACE]:" +p >control

Trivially, those sets can be subsected with the other query terms too,
should the author see fit.

Using ddebug_exec_queries() from a callback, authors can change
*pr_debug* callstates when drm.debug is updated from userspace.  They
can map bits, or strings, or whatever they want.

They could even alter [fmlt] flags, though I dont foresee why they would.

Is it safe ?

ddebug_exec_queries() is currently 'exposed' to user space in
several limited ways;

1 it is called from module-load callback, where it implements the
  $modname.dyndbg=+p "fake" parameter provided to all modules.

2 it handles query input from >control directly

IOW, it is "fully" exposed to local root user; exposing the same
functionality to other kernel modules is no additional risk.

The other big issue to check is locking:

dyndbg has a single mutex, taken by ddebug_change to handle >control,
and by ddebug_proc_(start|stop) to span `cat control`.  Queries
submitted via export will have module specified, which dramatically
cuts matching work done by ddebug_change vs "module=* +p".
ISTM this proposed export presents no locking problems.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 65c224301509..b00f536d6d12 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -546,6 +546,7 @@ static int ddebug_exec_queries(char *query, const char *modname)
 		return exitcode;
 	return nfound;
 }
+EXPORT_SYMBOL_GPL(ddebug_exec_queries);
 
 #define PREFIX_SIZE 64
 
-- 
2.26.2