linux-kernel - Re: [PATCH] trace: Return ENOTCONN instead of EBADF

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

Message-ID: <20201012095324.78996fd2@gandalf.local.home>
Date:   Mon, 12 Oct 2020 09:53:24 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Peter Enderborg <peter.enderborg@...y.com>
Cc:     <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH] trace: Return ENOTCONN instead of EBADF

On Mon, 12 Oct 2020 10:26:42 +0200
Peter Enderborg <peter.enderborg@...y.com> wrote:

> When there is no clients listening on event the trace return
> EBADF. The file is not a bad file descriptor and to get the
> userspace able to do a proper error handling it need a different
> error code that separate a bad file descriptor from a empty listening.

I have no problem with this patch, but your description is incorrect. And
before making this change, I want to make sure that what you think is
happening is actually happening.

This has nothing to do with "clients listening". This happens when the ring
buffer is disabled for some reason. The most likely case of this happening
is if someone sets /sys/kernel/tracing/tracing_on to zero.

If this is still something you want applied, please update the change log
to a more accurate scenario.

Thanks,

-- Steve


> 
> Signed-off-by: Peter Enderborg <peter.enderborg@...y.com>
> ---
>  kernel/trace/trace.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d3e5de717df2..6e592bf736df 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6651,8 +6651,8 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
>  	event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size,
>  					    irq_flags, preempt_count());
>  	if (unlikely(!event))
> -		/* Ring buffer disabled, return as if not open for write */
> -		return -EBADF;
> +		/* Ring buffer disabled, return as if not connected */
> +		return -ENOTCONN;
>  
>  	entry = ring_buffer_event_data(event);
>  	entry->ip = _THIS_IP_;
> @@ -6731,8 +6731,8 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
>  	event = __trace_buffer_lock_reserve(buffer, TRACE_RAW_DATA, size,
>  					    irq_flags, preempt_count());
>  	if (!event)
> -		/* Ring buffer disabled, return as if not open for write */
> -		return -EBADF;
> +		/* Ring buffer disabled, return not connected */
> +		return -ENOTCONN;
>  
>  	entry = ring_buffer_event_data(event);
>