linux-kernel - Re: [PATCH v7 1/5] tracing/probes: support '%pd' type for print struct dentry's name

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: <20240321101547.1f4e68ab@gandalf.local.home>
Date: Thu, 21 Mar 2024 10:15:47 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Ye Bin <yebin10@...wei.com>
Cc: <mhiramat@...nel.org>, <mathieu.desnoyers@...icios.com>,
 <linux-trace-kernel@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 1/5] tracing/probes: support '%pd' type for print
 struct dentry's name

On Wed, 20 Mar 2024 21:29:20 +0800
Ye Bin <yebin10@...wei.com> wrote:

> Support print type '%pd' for print dentry's  name.
> 

The above is not a very detailed change log. A change log should state not
only what the change is doing, but also why.

Having examples of before and after would be useful in the change log.

> Signed-off-by: Ye Bin <yebin10@...wei.com>
> ---
>  kernel/trace/trace.c        |  2 +-
>  kernel/trace/trace_fprobe.c |  6 +++++
>  kernel/trace/trace_kprobe.c |  6 +++++
>  kernel/trace/trace_probe.c  | 50 +++++++++++++++++++++++++++++++++++++
>  kernel/trace/trace_probe.h  |  2 ++
>  5 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b12f8384a36a..ac26b8446337 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c


> +/* @buf: *buf must be equal to NULL. Caller must to free *buf */
> +int traceprobe_expand_dentry_args(int argc, const char *argv[], char **buf)
> +{
> +	int i, used, ret;
> +	const int bufsize = MAX_DENTRY_ARGS_LEN;
> +	char *tmpbuf = NULL;
> +
> +	if (*buf)
> +		return -EINVAL;
> +
> +	used = 0;
> +	for (i = 0; i < argc; i++) {
> +		if (glob_match("*:%pd", argv[i])) {
> +			char *tmp;
> +			char *equal;
> +
> +			if (!tmpbuf) {
> +				tmpbuf = kmalloc(bufsize, GFP_KERNEL);
> +				if (!tmpbuf)
> +					return -ENOMEM;
> +			}
> +
> +			tmp = kstrdup(argv[i], GFP_KERNEL);
> +			if (!tmp)
> +				goto nomem;
> +
> +			equal = strchr(tmp, '=');
> +			if (equal)
> +				*equal = '\0';
> +			tmp[strlen(argv[i]) - 4] = '\0';
> +			ret = snprintf(tmpbuf + used, bufsize - used,
> +				       "%s%s+0x0(+0x%zx(%s)):string",
> +				       equal ? tmp : "", equal ? "=" : "",
> +				       offsetof(struct dentry, d_name.name),
> +				       equal ? equal + 1 : tmp);

What would be really useful is if we had a way to expose BTF here. Something like:

 "%pB:<struct>:<field>"

The "%pB" would mean to look up the struct/field offsets and types via BTF,
and create the appropriate command to find and print it.

That would be much more flexible and useful as it would allow reading any
field from any structure without having to use gdb.

-- Steve


> +			kfree(tmp);
> +			if (ret >= bufsize - used)
> +				goto nomem;
> +			argv[i] = tmpbuf + used;
> +			used += ret + 1;
> +		}
> +	}
> +
> +	*buf = tmpbuf;
> +	return 0;
> +nomem:
> +	kfree(tmpbuf);
> +	return -ENOMEM;
> +}