linux-kernel - Re: [GIT PULL v2] tracing: tracing: user

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] [day] [month] [year] [list]

Message-ID: <20230628105839.68cd8780@rorschach.local.home>
Date:   Wed, 28 Jun 2023 10:58:39 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        sunliming <sunliming@...inos.cn>,
        Beau Belgrave <beaub@...ux.microsoft.com>
Subject: Re: [GIT PULL v2] tracing: tracing: user_event fix for 6.4

Linus,

Hold off on pulling this. I'll make a v3 to include a stable and Fixes
tag as this didn't make it into the release that the code was added.

-- Steve


On Wed, 28 Jun 2023 10:54:25 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> Linus,
> 
> tracing: Fix user event write on buffer disabled
> 
> The user events write currently returns the size of what was suppose to be
> written when tracing is disabled and nothing was written. Instead, behave like
> trace_marker and return -EBADF, as that is what is returned if a file is opened
> for read only, and a write is performed on it. Writing to the buffer
> that is disabled is like trying to write to a file opened for read
> only, as the buffer still can be read, but just not written to.
> 
> This also includes test cases for this use case.
> 
> 
> Please pull the latest trace-v6.4-rc7-v2 tree, which can be found at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
> trace-v6.4-rc7-v2
> 
> Tag SHA1: 02efa9f9b19b0d881812bae9b3e28d539b67a863
> Head SHA1: e155047e53d25f09d055c08ae9d6c269520e90d8
> 
> 
> sunliming (3):
>       tracing/user_events: Fix incorrect return value for writing operation when events are disabled
>       selftests/user_events: Enable the event before write_fault test in ftrace self-test
>       selftests/user_events: Add test cases when event is disabled
> 
> ----
>  kernel/trace/trace_events_user.c                  | 3 ++-
>  tools/testing/selftests/user_events/ftrace_test.c | 8 ++++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> ---------------------------
> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 8df0550415e7..09f7d9167b8e 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -2096,7 +2096,8 @@ static ssize_t user_events_write_core(struct file *file, struct iov_iter *i)
>  
>  		if (unlikely(faulted))
>  			return -EFAULT;
> -	}
> +	} else
> +		return -EBADF;
>  
>  	return ret;
>  }
> diff --git a/tools/testing/selftests/user_events/ftrace_test.c b/tools/testing/selftests/user_events/ftrace_test.c
> index eb6904d89f14..5beb0aef1d81 100644
> --- a/tools/testing/selftests/user_events/ftrace_test.c
> +++ b/tools/testing/selftests/user_events/ftrace_test.c
> @@ -324,6 +324,10 @@ TEST_F(user, write_events) {
>  	io[0].iov_base = &reg.write_index;
>  	io[0].iov_len = sizeof(reg.write_index);
>  
> +	/* Write should return -EBADF when event is not enabled */
> +	ASSERT_EQ(-1, writev(self->data_fd, (const struct iovec *)io, 3));
> +	ASSERT_EQ(EBADF, errno);
> +
>  	/* Enable event */
>  	self->enable_fd = open(enable_file, O_RDWR);
>  	ASSERT_NE(-1, write(self->enable_fd, "1", sizeof("1")))
> @@ -400,6 +404,10 @@ TEST_F(user, write_fault) {
>  	ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, &reg));
>  	ASSERT_EQ(0, reg.write_index);
>  
> +	/* Enable event */
> +	self->enable_fd = open(enable_file, O_RDWR);
> +	ASSERT_NE(-1, write(self->enable_fd, "1", sizeof("1")))
> +
>  	/* Write should work normally */
>  	ASSERT_NE(-1, writev(self->data_fd, (const struct iovec *)io, 2));
>