Skip to contentSkip to conversation

tracing: More fixes for v6.7-rc6

PATCH New Steven Rostedt 1 participant 4 messages Dec 21, 2023

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover tracing: More fixes for v6.7-rc6

Tracing fixes for 6.7:

- Fix another kerneldoc warning

- Fix eventfs files to inherit the ownership of its parent directory. The dynamic creating of dentries in eventfs did not take into account if the tracefs file system was mounted with a gid/uid, and would still default to the gid/uid of root. This is a regression.

- Fix warning when synthetic event testing is enabled along with startup event tracing testing is enabled

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/urgent

Head SHA1: 88b30c7f5d27e1594d70dc2bd7199b18f2b57fa9

Randy Dunlap (1):
tracing/synthetic: fix kernel-doc warnings

Steven Rostedt (Google) (2):
eventfs: Have event files and directories default to parent uid and gid tracing / synthetic: Disable events after testing in synth_event_gen_test_init()

----

 fs/tracefs/event_inode.c            | 12 +++++++++---
 kernel/trace/synth_event_gen_test.c | 11 +++++++++++
 kernel/trace/trace_events_synth.c   |  4 ++--
 3 files changed, 22 insertions(+), 5 deletions(-)
author maintainer
#1 lore raw

PATCH 1/3 tracing/synthetic: fix kernel-doc warnings

From: Randy Dunlap <[email protected]>

scripts/kernel-doc warns about using @args: for variadic arguments to functions. Documentation/doc-guide/kernel-doc.rst says that this should be written as @...: instead, so update the source code to match that, preventing the warnings.

trace_events_synth.c:1165: warning: Excess function parameter 'args' description in '__synth_event_gen_cmd_start' trace_events_synth.c:1714: warning: Excess function parameter 'args' description in 'synth_event_trace'

Cc Mathieu Desnoyers <[email protected]>
Fixes 35ca5207c2d11 ("tracing: Add synthetic event command generation functions")
Fixes 8dcc53ad956d2 ("tracing: Add synth_event_trace() and related functions")
---
 kernel/trace/trace_events_synth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/trace/trace_events_synth.c +2 -2
index 846e02c0fb59..e7af286af4f1 100644
@@ -1137,7 +1137,7 @@ EXPORT_SYMBOL_GPL(synth_event_add_fields);
  * @cmd: A pointer to the dynevent_cmd struct representing the new event
  * @name: The name of the synthetic event
  * @mod: The module creating the event, NULL if not created from a module
- * @args: Variable number of arg (pairs), one pair for each field
+ * @...: Variable number of arg (pairs), one pair for each field
  *
  * NOTE: Users normally won't want to call this function directly, but
  * rather use the synth_event_gen_cmd_start() wrapper, which
@@ -1695,7 +1695,7 @@ __synth_event_trace_end(struct synth_event_trace_state *trace_state)
  * synth_event_trace - Trace a synthetic event
  * @file: The trace_event_file representing the synthetic event
  * @n_vals: The number of values in vals
- * @args: Variable number of args containing the event values
+ * @...: Variable number of args containing the event values
  *
  * Trace a synthetic event using the values passed in the variable
  * argument list.
Signature · 1 line
2.42.0
author maintainer
#2 lore raw

PATCH 2/3 eventfs: Have event files and directories default to parent uid and gid

In reply to Steven Rostedt

From: "Steven Rostedt (Google)" <[email protected]>

Dongliang reported:

I found that in the latest version, the nodes of tracefs have been changed to dynamically created.

This has caused me to encounter a problem where the gid I specified in the mounting parameters cannot apply to all files, as in the following situation:

/data/tmp/events # mount | grep tracefs
tracefs on /data/tmp type tracefs (rw,seclabel,relatime,gid=3012)

gid 3012 = readtracefs

/data/tmp # ls -lh
total 0
-r--r----- 1 root readtracefs 0 1970-01-01 08:00 README
-r--r----- 1 root readtracefs 0 1970-01-01 08:00 available_events

ums9621_1h10:/data/tmp/events # ls -lh total 0
drwxr-xr-x 2 root root 0 2023-12-19 00:56 alarmtimer drwxr-xr-x 2 root root 0 2023-12-19 00:56 asoc

It will prevent certain applications from accessing tracefs properly, I try to avoid this issue by making the following modifications.

To fix this, have the files created default to taking the ownership of the parent dentry unless the ownership was previously set by the user.

Cc Mathieu Desnoyers <[email protected]>
Cc Hongyu Jin <[email protected]>
Fixes 28e12c09f5aa0 ("eventfs: Save ownership and mode")
---
 fs/tracefs/event_inode.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)
fs/tracefs/event_inode.c +9 -3
index 43e237864a42..2ccc849a5bda 100644
@@ -148,7 +148,8 @@ static const struct file_operations eventfs_file_operations = {
 	.release	= eventfs_release,
 };
 
-static void update_inode_attr(struct inode *inode, struct eventfs_attr *attr, umode_t mode)
+static void update_inode_attr(struct dentry *dentry, struct inode *inode,
+			      struct eventfs_attr *attr, umode_t mode)
 {
 	if (!attr) {
 		inode->i_mode = mode;
@@ -162,9 +163,13 @@ static void update_inode_attr(struct inode *inode, struct eventfs_attr *attr, um
 
 	if (attr->mode & EVENTFS_SAVE_UID)
 		inode->i_uid = attr->uid;
+	else
+		inode->i_uid = d_inode(dentry->d_parent)->i_uid;
 
 	if (attr->mode & EVENTFS_SAVE_GID)
 		inode->i_gid = attr->gid;
+	else
+		inode->i_gid = d_inode(dentry->d_parent)->i_gid;
 }
 
 /**
@@ -206,7 +211,7 @@ static struct dentry *create_file(const char *name, umode_t mode,
 		return eventfs_failed_creating(dentry);
 
 	/* If the user updated the directory's attributes, use them */
-	update_inode_attr(inode, attr, mode);
+	update_inode_attr(dentry, inode, attr, mode);
 
 	inode->i_op = &eventfs_file_inode_operations;
 	inode->i_fop = fop;
@@ -242,7 +247,8 @@ static struct dentry *create_dir(struct eventfs_inode *ei, struct dentry *parent
 		return eventfs_failed_creating(dentry);
 
 	/* If the user updated the directory's attributes, use them */
-	update_inode_attr(inode, &ei->attr, S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO);
+	update_inode_attr(dentry, inode, &ei->attr,
+			  S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO);
 
 	inode->i_op = &eventfs_root_dir_inode_operations;
 	inode->i_fop = &eventfs_file_operations;
Signature · 1 line
2.42.0
author maintainer
#3 lore raw

PATCH 3/3 tracing / synthetic: Disable events after testing in synth_event_gen_test_init()

In reply to Steven Rostedt

From: "Steven Rostedt (Google)" <[email protected]>

The synth_event_gen_test module can be built in, if someone wants to run the tests at boot up and not have to load them.

The synth_event_gen_test_init() function creates and enables the synthetic events and runs its tests.

The synth_event_gen_test_exit() disables the events it created and destroys the events.

If the module is builtin, the events are never disabled. The issue is, the events should be disable after the tests are run. This could be an issue if the rest of the boot up tests are enabled, as they expect the events to be in a known state before testing. That known state happens to be disabled.

When CONFIG_SYNTH_EVENT_GEN_TEST=y and CONFIG_EVENT_TRACE_STARTUP_TEST=y a warning will trigger:

Running tests on trace events:
Testing event create_synth_test:
Enabled event during self test!

 ------------[ cut here ]------------
 WARNING: CPU: 2 PID: 1 at kernel/trace/trace_events.c:4150 event_trace_self_tests+0x1c2/0x480
 Modules linked in:
 CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.7.0-rc2-test-00031-gb803d7c664d5-dirty #276
 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.2-debian-1.16.2-1 04/01/2014
 RIP: 0010:event_trace_self_tests+0x1c2/0x480
 Code: bb e8 a2 ab 5d fc 48 8d 7b 48 e8 f9 3d 99 fc 48 8b 73 48 40 f6 c6 01 0f 84 d6 fe ff ff 48 c7 c7 20 b6 ad bb e8 7f ab 5d fc 90 <0f> 0b 90 48 89 df e8 d3 3d 99 fc 48 8b 1b 4c 39 f3 0f 85 2c ff ff
 RSP: 0000:ffffc9000001fdc0 EFLAGS: 00010246
 RAX: 0000000000000029 RBX: ffff88810399ca80 RCX: 0000000000000000
 RDX: 0000000000000000 RSI: ffffffffb9f19478 RDI: ffff88823c734e64
 RBP: ffff88810399f300 R08: 0000000000000000 R09: fffffbfff79eb32a
 R10: ffffffffbcf59957 R11: 0000000000000001 R12: ffff888104068090
 R13: ffffffffbc89f0a0 R14: ffffffffbc8a0f08 R15: 0000000000000078
 FS:  0000000000000000(0000) GS:ffff88823c700000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000000 CR3: 00000001f6282001 CR4: 0000000000170ef0
 Call Trace:
  <TASK>
  ? __warn+0xa5/0x200
  ? event_trace_self_tests+0x1c2/0x480
  ? report_bug+0x1f6/0x220
  ? handle_bug+0x6f/0x90
  ? exc_invalid_op+0x17/0x50
  ? asm_exc_invalid_op+0x1a/0x20
  ? tracer_preempt_on+0x78/0x1c0
  ? event_trace_self_tests+0x1c2/0x480
  ? __pfx_event_trace_self_tests_init+0x10/0x10
  event_trace_self_tests_init+0x27/0xe0
  do_one_initcall+0xd6/0x3c0
  ? __pfx_do_one_initcall+0x10/0x10
  ? kasan_set_track+0x25/0x30
  ? rcu_is_watching+0x38/0x60
  kernel_init_freeable+0x324/0x450
  ? __pfx_kernel_init+0x10/0x10
  kernel_init+0x1f/0x1e0
  ? _raw_spin_unlock_irq+0x33/0x50
  ret_from_fork+0x34/0x60
  ? __pfx_kernel_init+0x10/0x10
  ret_from_fork_asm+0x1b/0x30
  </TASK>

This is because the synth_event_gen_test_init() left the synthetic events that it created enabled. By having it disable them after testing, the other selftests will run fine.

Cc Mathieu Desnoyers <[email protected]>
Cc Tom Zanussi <[email protected]>
Fixes 9fe41efaca084 ("tracing: Add synth event generation test module")
---
 kernel/trace/synth_event_gen_test.c | 11 +++++++++++
 1 file changed, 11 insertions(+)
kernel/trace/synth_event_gen_test.c +11 -0
index 8dfe85499d4a..354c2117be43 100644
@@ -477,6 +477,17 @@ static int __init synth_event_gen_test_init(void)
 
 	ret = test_trace_synth_event();
 	WARN_ON(ret);
+
+	/* Disable when done */
+	trace_array_set_clr_event(gen_synth_test->tr,
+				  "synthetic",
+				  "gen_synth_test", false);
+	trace_array_set_clr_event(empty_synth_test->tr,
+				  "synthetic",
+				  "empty_synth_test", false);
+	trace_array_set_clr_event(create_synth_test->tr,
+				  "synthetic",
+				  "create_synth_test", false);
  out:
 	return ret;
 }
Signature · 1 line
2.42.0