All of lore.kernel.org
 help / color / mirror / Atom feed
From: Beau Belgrave <beaub@linux.microsoft.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: mhiramat@kernel.org, linux-trace-devel@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v6 02/13] user_events: Add minimal support for trace_event into ftrace
Date: Thu, 9 Dec 2021 12:11:12 -0800	[thread overview]
Message-ID: <20211209201112.GB21676@kbox> (raw)
In-Reply-To: <20211209145738.4da346ba@gandalf.local.home>

On Thu, Dec 09, 2021 at 02:57:38PM -0500, Steven Rostedt wrote:
> On Thu, 9 Dec 2021 11:42:35 -0800
> Beau Belgrave <beaub@linux.microsoft.com> wrote:
> 
> > User program task:
> > CPU0: ioctl(fd, REG)
> > CPU1: close(fd)
> > 
> > IE: Some program registers and then immediately calls close on the file.
> > If the CPU migrates right between the 2 and the close swaps, it is
> > possible this could occur.
> > 
> > This could be attempted in tight loops maliciously as well.
> > 
> > I assume with a mutex there that some barrier is imposed to ensure
> > correct reads in this condition? (By virtue of the mutex acquire/check)
> 
> But as I stated before, the ioctl() uses fdget() which will prevent he
> close from calling the release. Even if they get swapped. If close goes
> first and starts down the path of the release, then the ioctl is guaranteed
> to return -EBADF. If it gets the fd, then close will be a nop, and the exit
> of the ioctl will call the release.
> 
> If this wasn't the case, then the race I was concerned about would be an
> issue.
> 
> Because we are both confused by this, add the mutex! :-)
> 
> -- Steve

Agreed, I will add the mutex. :)

I guess I am being paranoid about an architecture that does not have
automatic cache consistency and while the write / read don't happen at
the exact time, they happen close together. Close enough that one CPU
reads the old value from a cache line and gets it wrong.

I don't believe that is possible on Intel, but I don't know if it's
possible on other architectures (especially older ones).

Thanks,
-Beau

  reply	other threads:[~2021-12-09 20:11 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-01 18:25 [PATCH v6 00/13] user_events: Enable user processes to create and write to trace events Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 01/13] user_events: Add UABI header for user access to user_events Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 02/13] user_events: Add minimal support for trace_event into ftrace Beau Belgrave
2021-12-08 23:19   ` Steven Rostedt
2021-12-09  0:58     ` Beau Belgrave
2021-12-09  2:03       ` Steven Rostedt
2021-12-09 17:40         ` Beau Belgrave
2021-12-09 17:47           ` Steven Rostedt
2021-12-09 19:42             ` Beau Belgrave
2021-12-09 19:57               ` Steven Rostedt
2021-12-09 20:11                 ` Beau Belgrave [this message]
2021-12-09 20:19                   ` Steven Rostedt
2021-12-01 18:25 ` [PATCH v6 03/13] user_events: Add print_fmt generation support for basic types Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 04/13] user_events: Handle matching arguments from dyn_events Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 05/13] user_events: Add basic perf and eBPF support Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 06/13] user_events: Add self-test for ftrace integration Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 07/13] user_events: Add self-test for dynamic_events integration Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 08/13] user_events: Add self-test for perf_event integration Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 09/13] user_events: Optimize writing events by only copying data once Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 10/13] user_events: Add documentation file Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 11/13] user_events: Add sample code for typical usage Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 12/13] user_events: Validate user payloads for size and null termination Beau Belgrave
2021-12-01 18:25 ` [PATCH v6 13/13] user_events: Use __get_rel_str for relative string fields Beau Belgrave

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=20211209201112.GB21676@kbox \
    --to=beaub@linux.microsoft.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.