Re: [GIT PULL v2] tracing: tracing: user_event fix for 6.4

[Steven Rostedt]

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@xxxxxxxxxxx> 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));
>