[PATCH] libtracefs: Pass the pipe size by value to F_SETPIPE_SZ

[PATCH] libtracefs: Pass the pipe size by value to F_SETPIPE_SZ

[Li Pengfei]

F_SETPIPE_SZ takes the new size as the argument itself, but init_splice()
passes a pointer to it.  The kernel takes that argument as an unsigned int,
so the pointer is truncated to its low 32 bits and used as the size.  That
usually fails, leaving the pipe at the default 64K, so a sub buffer larger
than that no longer fits in one splice() and the recorders lose data.  It
can also succeed at an unrelated size with no error to notice.

Failing here stays non fatal, as an unprivileged caller cannot raise a pipe
above /proc/sys/fs/pipe-max-size and the default size still records.

While here, drop the third argument from the F_GETPIPE_SZ call next to it.
That command takes no argument and returns the size, so the pointer was
never read and the return value was already being used.  The branch it fed
was unreachable: F_GETPIPE_SZ never returns zero on success.

Signed-off-by: Li Pengfei <lipengfei28@xiaomi.com>
---
 src/tracefs-record.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/tracefs-record.c b/src/tracefs-record.c
index 21a2f6b..9ef13e0 100644
--- a/src/tracefs-record.c
+++ b/src/tracefs-record.c
@@ -554,20 +554,22 @@ static int init_splice(struct tracefs_cpu *tcpu)
 
 	if (tfs_str_read_file("/proc/sys/fs/pipe-max-size", &buf, false) > 0) {
 		int size = atoi(buf);
-		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, &size);
+
+		/* F_SETPIPE_SZ takes the size itself, not a pointer to it */
+		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, size);
 		free(buf);
 	}
 
-	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ, &tcpu->pipe_size);
+	/* F_GETPIPE_SZ returns the size, it takes no third argument */
+	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ);
 	/*
 	 * F_GETPIPE_SZ was introduced in 2.6.35, ftrace was introduced
 	 * in 2.6.31. If we are running on an older kernel, just fall
-	 * back to using subbuf_size for splice(). It could also return
-	 * the size of the pipe and not set pipe_size.
+	 * back to using subbuf_size for splice().
 	 */
-	if (ret > 0 && !tcpu->pipe_size)
+	if (ret > 0)
 		tcpu->pipe_size = ret;
-	else if (ret < 0)
+	else
 		tcpu->pipe_size = tcpu->subbuf_size;
 
 	tcpu->splice_read_flags = SPLICE_F_MOVE;
-- 
2.34.1

Re: [PATCH] libtracefs: Pass the pipe size by value to F_SETPIPE_SZ

[Steven Rostedt]

On Sat,  7 Nov 2026 22:07:55 +0800
Li Pengfei <ljdlns1987@gmail.com> wrote:

> F_SETPIPE_SZ takes the new size as the argument itself, but init_splice()
> passes a pointer to it.  The kernel takes that argument as an unsigned int,
> so the pointer is truncated to its low 32 bits and used as the size.  That
> usually fails, leaving the pipe at the default 64K, so a sub buffer larger
> than that no longer fits in one splice() and the recorders lose data.  It
> can also succeed at an unrelated size with no error to notice.
> 
> Failing here stays non fatal, as an unprivileged caller cannot raise a pipe
> above /proc/sys/fs/pipe-max-size and the default size still records.

Good catch!

> 
> While here, drop the third argument from the F_GETPIPE_SZ call next to it.
> That command takes no argument and returns the size, so the pointer was
> never read and the return value was already being used.  The branch it fed
> was unreachable: F_GETPIPE_SZ never returns zero on success.
> 
> Signed-off-by: Li Pengfei <lipengfei28@xiaomi.com>
> ---
>  src/tracefs-record.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/src/tracefs-record.c b/src/tracefs-record.c
> index 21a2f6b..9ef13e0 100644
> --- a/src/tracefs-record.c
> +++ b/src/tracefs-record.c
> @@ -554,20 +554,22 @@ static int init_splice(struct tracefs_cpu *tcpu)
>  
>  	if (tfs_str_read_file("/proc/sys/fs/pipe-max-size", &buf, false) > 0) {
>  		int size = atoi(buf);
> -		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, &size);
> +
> +		/* F_SETPIPE_SZ takes the size itself, not a pointer to it */

Nit. You don't need to add comments to bug fixes. That is, the above
comment is useless.

Thanks,

-- Steve



> +		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, size);
>  		free(buf);
>  	}
>  
> -	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ, &tcpu->pipe_size);
> +	/* F_GETPIPE_SZ returns the size, it takes no third argument */
> +	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ);
>  	/*
>  	 * F_GETPIPE_SZ was introduced in 2.6.35, ftrace was introduced
>  	 * in 2.6.31. If we are running on an older kernel, just fall
> -	 * back to using subbuf_size for splice(). It could also return
> -	 * the size of the pipe and not set pipe_size.
> +	 * back to using subbuf_size for splice().
>  	 */
> -	if (ret > 0 && !tcpu->pipe_size)
> +	if (ret > 0)
>  		tcpu->pipe_size = ret;
> -	else if (ret < 0)
> +	else
>  		tcpu->pipe_size = tcpu->subbuf_size;
>  
>  	tcpu->splice_read_flags = SPLICE_F_MOVE;

Re: [PATCH] libtracefs: Pass the pipe size by value to F_SETPIPE_SZ

[Steven Rostedt]

On Sat, 7 Nov 2026 17:40:11 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> >  	if (tfs_str_read_file("/proc/sys/fs/pipe-max-size", &buf, false) > 0) {
> >  		int size = atoi(buf);
> > -		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, &size);
> > +
> > +		/* F_SETPIPE_SZ takes the size itself, not a pointer to it */  
> 
> Nit. You don't need to add comments to bug fixes. That is, the above
> comment is useless.

Let me rephrase that. You can add comments to bug fixes. What I meant
was, the above comment is basically stating what was fixed, which is
useless for those looking at the code after the update.

-- Steve

> 
> 
> > +		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, size);
> >  		free(buf);
> >  	}

[PATCH v2] libtracefs: Pass the pipe size by value to F_SETPIPE_SZ

[Li Pengfei]

F_SETPIPE_SZ takes the new size as the argument itself, but init_splice()
passes a pointer to it.  The kernel takes that argument as an unsigned int,
so the pointer is truncated to its low 32 bits and used as the size.  That
usually fails, leaving the pipe at the default 64K, so a sub buffer larger
than that no longer fits in one splice() and the recorders lose data.  It
can also succeed at an unrelated size with no error to notice.

Failing here stays non fatal, as an unprivileged caller cannot raise a pipe
above /proc/sys/fs/pipe-max-size and the default size still records.

While here, drop the third argument from the F_GETPIPE_SZ call next to it.
That command takes no argument and returns the size, so the pointer was
never read and the return value was already being used.  The branch it fed
was unreachable: F_GETPIPE_SZ never returns zero on success.

Fixes: e34cbd83f6d3 ("libtracefs: Increase splice to use pipe max size")
Signed-off-by: Li Pengfei <lipengfei28@xiaomi.com>
---
Changes since v1:
- Drop both comments.  They described what the fix changed rather than
  anything the code needs explained afterwards.
- Add the Fixes tag, which v1 was missing.

 src/tracefs-record.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/src/tracefs-record.c b/src/tracefs-record.c
index 21a2f6b..f5197c4 100644
--- a/src/tracefs-record.c
+++ b/src/tracefs-record.c
@@ -554,20 +554,19 @@ static int init_splice(struct tracefs_cpu *tcpu)
 
 	if (tfs_str_read_file("/proc/sys/fs/pipe-max-size", &buf, false) > 0) {
 		int size = atoi(buf);
-		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, &size);
+		fcntl(tcpu->splice_pipe[0], F_SETPIPE_SZ, size);
 		free(buf);
 	}
 
-	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ, &tcpu->pipe_size);
+	ret = fcntl(tcpu->splice_pipe[0], F_GETPIPE_SZ);
 	/*
 	 * F_GETPIPE_SZ was introduced in 2.6.35, ftrace was introduced
 	 * in 2.6.31. If we are running on an older kernel, just fall
-	 * back to using subbuf_size for splice(). It could also return
-	 * the size of the pipe and not set pipe_size.
+	 * back to using subbuf_size for splice().
 	 */
-	if (ret > 0 && !tcpu->pipe_size)
+	if (ret > 0)
 		tcpu->pipe_size = ret;
-	else if (ret < 0)
+	else
 		tcpu->pipe_size = tcpu->subbuf_size;
 
 	tcpu->splice_read_flags = SPLICE_F_MOVE;
-- 
2.34.1