public inbox for linux-trace-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH v6 2/4] tracing: Make the backup instance non-reusable
Date: Thu, 12 Feb 2026 13:24:06 +0900	[thread overview]
Message-ID: <20260212132406.69813d1f0ffaa3fdb19f9c40@kernel.org> (raw)
In-Reply-To: <20260211104244.193953be@fedora>

On Wed, 11 Feb 2026 10:42:44 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 10 Feb 2026 14:14:15 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > Hmm, OK. Now I found how sysfs handles it.
> > 
> > 	/*
> > 	 * For regular files, if the opener has CAP_DAC_OVERRIDE, open(2)
> > 	 * succeeds regardless of the RW permissions.  sysfs had an extra
> > 	 * layer of enforcement where open(2) fails with -EACCES regardless
> > 	 * of CAP_DAC_OVERRIDE if the permission doesn't have the
> > 	 * respective read or write access at all (none of S_IRUGO or
> > 	 * S_IWUGO) or the respective operation isn't implemented.  The
> > 	 * following flag enables that behavior.
> > 	 */
> > 	KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK	= 0x0002,
> > 
> > So for the similar reason, I will make tracefs to check the permission
> > even if CAP_DAC_OVERRIDE is set. (But this check should be done in general,
> > instead of each open() operation)
> > 
> 
> I don't believe this is the same. This is about an instance being truly
> read only. The instance is special, not the files. Note, permissions can
> be changed by root too.

Ah, OK. Let me add read only checks in all related .open operations.

> After applying your patches, I did the following:
> 
> ~# cd /sys/kernel/tracing/instances/backup/
> ~# ls -l current_tracer
> -r--r----- 1 root root 0 Feb 11 10:29 current_tracer
> 
> ~# cat current_tracer 
> nop
> 
> ~# cat trace
> # tracer: nop
> #
> # entries-in-buffer/entries-written: 0/0   #P:8
> #
> #                                _-----=> irqs-off/BH-disabled
> #                               / _----=> need-resched
> #                              | / _---=> hardirq/softirq
> #                              || / _--=> preempt-depth
> #                              ||| / _-=> migrate-disable
> #                              |||| /     delay
> #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> #              | |         |   |||||     |         |
> 
> ~# chmod 664 current_tracer
> ~# ls -l current_tracer
> -rw-rw-r-- 1 root root 0 Feb 11 10:29 current_tracer

Ah, OK...

> 
> ~# echo function > current_tracer
> ~# cat current_tracer 
> function
> 
> ~# cat trace
> # tracer: function
> #
> # entries-in-buffer/entries-written: 1750306/2076556   #P:8
> #
> #                                _-----=> irqs-off/BH-disabled
> #                               / _----=> need-resched
> #                              | / _---=> hardirq/softirq
> #                              || / _--=> preempt-depth
> #                              ||| / _-=> migrate-disable
> #                              |||| /     delay
> #           TASK-PID     CPU#  |||||  TIMESTAMP  FUNCTION
> #              | |         |   |||||     |         |
>             bash-1056    [001] .....   231.448852: mutex_unlock <-tracing_set_tracer
>           <idle>-0       [002] ...1.   231.448853: arch_cpu_idle_exit <-do_idle
> ##### CPU 7 buffer started ####
>           <idle>-0       [007] ...1.   231.448853: arch_cpu_idle_exit <-do_idle
>             bash-1056    [001] .....   231.448854: __mutex_unlock_slowpath <-tracing_set_tracer
>           <idle>-0       [002] d..1.   231.448855: arch_cpu_idle_enter <-do_idle
>           <idle>-0       [007] d..1.   231.448855: arch_cpu_idle_enter <-do_idle
>           <idle>-0       [007] d..1.   231.448855: tsc_verify_tsc_adjust <-arch_cpu_idle_enter
>           <idle>-0       [002] d..1.   231.448855: tsc_verify_tsc_adjust <-arch_cpu_idle_enter
>             bash-1056    [001] d....   231.448856: fpregs_assert_state_consistent <-arch_exit_to_user_mode_prepare
>           <idle>-0       [007] d..1.   231.448856: local_touch_nmi <-do_idle
>           <idle>-0       [002] d..1.   231.448856: local_touch_nmi <-do_idle
>             bash-1056    [001] d....   231.448856: switch_fpu_return <-arch_exit_to_user_mode_prepare
>           <idle>-0       [007] d..1.   231.448856: rcu_nocb_flush_deferred_wakeup <-do_idle
>           <idle>-0       [002] d..1.   231.448856: rcu_nocb_flush_deferred_wakeup <-do_idle
>           <idle>-0       [007] d..1.   231.448857: cpuidle_get_cpu_driver <-do_idle
>           <idle>-0       [002] d..1.   231.448857: cpuidle_get_cpu_driver <-do_idle
>           <idle>-0       [007] d..1.   231.448857: cpuidle_not_available <-do_idle
> [..]
> 
> Not too read only!
> 
> I change permissions all the time for tracefs files, so I don't want
> that changed.
> 
> This is not the same as sysfs. Let's keep it simple. Have all the open
> callers that can do writes return error -EACCES if a file allows writes
> and is open for write, but is part of the read only instance.

OK.

Thank you,
> 
> -- Steve
>  


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2026-02-12  4:24 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-01  3:28 [PATCH v6 0/4] tracing: Remove backup instance after read all Masami Hiramatsu (Google)
2026-02-01  3:29 ` [PATCH v6 1/4] tracing: Reset last_boot_info if ring buffer is reset Masami Hiramatsu (Google)
2026-02-05  1:40   ` Steven Rostedt
2026-02-06 13:50     ` Masami Hiramatsu
2026-02-11 18:28       ` Steven Rostedt
2026-02-11 18:33         ` Steven Rostedt
2026-02-01  3:29 ` [PATCH v6 2/4] tracing: Make the backup instance non-reusable Masami Hiramatsu (Google)
2026-02-05  2:17   ` Steven Rostedt
2026-02-09  9:08     ` Masami Hiramatsu
2026-02-09 23:42       ` Steven Rostedt
2026-02-10  5:14         ` Masami Hiramatsu
2026-02-11 15:42           ` Steven Rostedt
2026-02-12  4:24             ` Masami Hiramatsu [this message]
2026-02-01  3:29 ` [PATCH v6 3/4] tracing: Remove the backup instance automatically after read Masami Hiramatsu (Google)
2026-02-01  3:29 ` [PATCH v6 4/4] tracing/Documentation: Add a section about backup instance Masami Hiramatsu (Google)
2026-02-05  0:41 ` [PATCH v6 0/4] tracing: Remove backup instance after read all Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260212132406.69813d1f0ffaa3fdb19f9c40@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox