[PATCH 0/3] Tidy up some client/server behaviors

[PATCH 0/3] Tidy up some client/server behaviors

[Eric Sandeen]

Someone got happy with AI scanning and reported a bunch of items
related to fio's client/server architecture - an architecture which
is neither authenticated nor encrypted, and which *by design* allows
arbitrary code execution, which is even documented in the man page.

A couple of the items stood out as probably worth fixing anyway, and
I figured it might be worth being more explicit about the security
posture of fio when the server is started listening on TCP, so that's
patch 3.

I'm not wedded to any of this, tbh, so Jens if you don't like any of
this I have no problem with just dropping any or all of these changes.

Thanks!

-Eric

[PATCH 1/3] client: sanitize FIO_NET_CMD_SENDFILE requests

[Eric Sandeen]

A legitimate fio server should only be requesting files from the
client's cwd; reject absolute paths or any paths above the cwd.

Since this adds yet another failure mode, move fail: to the end of
the function as is a bit more typical for a goto fail;

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 client.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/client.c b/client.c
index 374a744a..b32c2852 100644
--- a/client.c
+++ b/client.c
@@ -1809,12 +1809,18 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
 	size = sizeof(*rep);
 	rep = malloc(size);
 
+	/* Sanitize requested path to reject files not under cwd */
+	if (pdu->path[0] == '/' ||
+	    strstr((const char *)pdu->path, "..") != NULL) {
+		log_err("fio: rejecting sendfile request for unsafe path: %s\n",
+			(const char *)pdu->path);
+		rep->error = EPERM;
+		goto fail;
+	}
+
 	if (stat((char *)pdu->path, &sb) < 0) {
-fail:
 		rep->error = errno;
-		sendfile_reply(client->fd, rep, size, tag);
-		free(rep);
-		return 1;
+		goto fail;
 	}
 
 	size += sb.st_size;
@@ -1822,14 +1828,21 @@ fail:
 	rep->size = cpu_to_le32((uint32_t) sb.st_size);
 
 	fd = open((char *)pdu->path, O_RDONLY);
-	if (fd == -1 )
+	if (fd == -1) {
+		rep->error = errno;
 		goto fail;
+	}
 
 	rep->error = read_data(fd, &rep->data, sb.st_size);
 	sendfile_reply(client->fd, rep, size, tag);
 	free(rep);
 	close(fd);
 	return 0;
+
+fail:
+	sendfile_reply(client->fd, rep, size, tag);
+	free(rep);
+	return 1;
 }
 
 int fio_handle_client(struct fio_client *client)
-- 
2.55.0

Re: [PATCH 1/3] client: sanitize FIO_NET_CMD_SENDFILE requests

[Jens Axboe]

On 9/17/26 1:41 PM, Eric Sandeen wrote:
> A legitimate fio server should only be requesting files from the
> client's cwd; reject absolute paths or any paths above the cwd.
> 
> Since this adds yet another failure mode, move fail: to the end of
> the function as is a bit more typical for a goto fail;
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  client.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/client.c b/client.c
> index 374a744a..b32c2852 100644
> --- a/client.c
> +++ b/client.c
> @@ -1809,12 +1809,18 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
>  	size = sizeof(*rep);
>  	rep = malloc(size);
>  
> +	/* Sanitize requested path to reject files not under cwd */
> +	if (pdu->path[0] == '/' ||
> +	    strstr((const char *)pdu->path, "..") != NULL) {
> +		log_err("fio: rejecting sendfile request for unsafe path: %s\n",
> +			(const char *)pdu->path);
> +		rep->error = EPERM;
> +		goto fail;
> +	}

Won't this potentially break existing use cases?

-- 
Jens Axboe

Re: [PATCH 1/3] client: sanitize FIO_NET_CMD_SENDFILE requests

[Eric Sandeen]

On 9/17/26 3:00 PM, Jens Axboe wrote:
> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>> A legitimate fio server should only be requesting files from the
>> client's cwd; reject absolute paths or any paths above the cwd.
>>
>> Since this adds yet another failure mode, move fail: to the end of
>> the function as is a bit more typical for a goto fail;
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>  client.c | 23 ++++++++++++++++++-----
>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/client.c b/client.c
>> index 374a744a..b32c2852 100644
>> --- a/client.c
>> +++ b/client.c
>> @@ -1809,12 +1809,18 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
>>  	size = sizeof(*rep);
>>  	rep = malloc(size);
>>  
>> +	/* Sanitize requested path to reject files not under cwd */
>> +	if (pdu->path[0] == '/' ||
>> +	    strstr((const char *)pdu->path, "..") != NULL) {
>> +		log_err("fio: rejecting sendfile request for unsafe path: %s\n",
>> +			(const char *)pdu->path);
>> +		rep->error = EPERM;
>> +		goto fail;
>> +	}
> 
> Won't this potentially break existing use cases?
TBH I did this a while ago and never sent it, so let me re-check. I thought
I had considered that at the time...

OK yeah, I guess it is too broad, I suppose if a job name has ".." in it,
this would  cause improper rejections. I don't think anything legitimately
tries to grab files on relative paths, though.

I'm guessing you think this isn't worth it either, and if so again I'm
happy to just drop it. Or I could tighten it up to only catch things that
really try to grab arbitrary paths outside of cwd.

I'm also leaning towards not worth it. :)

-Eric

Re: [PATCH 1/3] client: sanitize FIO_NET_CMD_SENDFILE requests

[Jens Axboe]

On 9/17/26 2:46 PM, Eric Sandeen wrote:
> On 9/17/26 3:00 PM, Jens Axboe wrote:
>> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>>> A legitimate fio server should only be requesting files from the
>>> client's cwd; reject absolute paths or any paths above the cwd.
>>>
>>> Since this adds yet another failure mode, move fail: to the end of
>>> the function as is a bit more typical for a goto fail;
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>  client.c | 23 ++++++++++++++++++-----
>>>  1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/client.c b/client.c
>>> index 374a744a..b32c2852 100644
>>> --- a/client.c
>>> +++ b/client.c
>>> @@ -1809,12 +1809,18 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
>>>  	size = sizeof(*rep);
>>>  	rep = malloc(size);
>>>  
>>> +	/* Sanitize requested path to reject files not under cwd */
>>> +	if (pdu->path[0] == '/' ||
>>> +	    strstr((const char *)pdu->path, "..") != NULL) {
>>> +		log_err("fio: rejecting sendfile request for unsafe path: %s\n",
>>> +			(const char *)pdu->path);
>>> +		rep->error = EPERM;
>>> +		goto fail;
>>> +	}
>>
>> Won't this potentially break existing use cases?
> TBH I did this a while ago and never sent it, so let me re-check. I thought
> I had considered that at the time...
> 
> OK yeah, I guess it is too broad, I suppose if a job name has ".." in it,
> this would  cause improper rejections. I don't think anything legitimately
> tries to grab files on relative paths, though.
> 
> I'm guessing you think this isn't worth it either, and if so again I'm
> happy to just drop it. Or I could tighten it up to only catch things that
> really try to grab arbitrary paths outside of cwd.
> 
> I'm also leaning towards not worth it. :)

Not worth it.

-- 
Jens Axboe

[PATCH 2/3] client: don't send uninitialized data in fio_send_file error replies

[Eric Sandeen]

Various error cases in fio_send_file() might send an uninitialized
buffer over the wire. The server should ignore this when errno is set,
but it's best practice to not ship heap data over the wire, so fix that
by:

- using calloc vs malloc in the initial allocation so that early error
  paths send initialized data.

- grow the buffer via realloc only after open() succeeds, so that an
  open() failure doesn't send a grown/uninit buffer back with the error.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 client.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/client.c b/client.c
index b32c2852..a949b5a3 100644
--- a/client.c
+++ b/client.c
@@ -1807,7 +1807,7 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
 	int fd;
 
 	size = sizeof(*rep);
-	rep = malloc(size);
+	rep = calloc(1, size);
 
 	/* Sanitize requested path to reject files not under cwd */
 	if (pdu->path[0] == '/' ||
@@ -1823,16 +1823,15 @@ static int fio_send_file(struct fio_client *client, struct cmd_sendfile *pdu,
 		goto fail;
 	}
 
-	size += sb.st_size;
-	rep = realloc(rep, size);
-	rep->size = cpu_to_le32((uint32_t) sb.st_size);
-
 	fd = open((char *)pdu->path, O_RDONLY);
 	if (fd == -1) {
 		rep->error = errno;
 		goto fail;
 	}
 
+	size += sb.st_size;
+	rep = realloc(rep, size);
+	rep->size = cpu_to_le32((uint32_t) sb.st_size);
 	rep->error = read_data(fd, &rep->data, sb.st_size);
 	sendfile_reply(client->fd, rep, size, tag);
 	free(rep);
-- 
2.55.0

[PATCH 3/3] fio: server: Warn when listening on network

[Eric Sandeen]

Make it crystal clear to users at run time that the client/server
protocol should only be used in trusted environments.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---
 server.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/server.c b/server.c
index 34f3f5c0..887ed7ed 100644
--- a/server.c
+++ b/server.c
@@ -2596,6 +2596,13 @@ static int fio_init_server_connection(void)
 	} else
 		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
 
+	if (!bind_sock) {
+		log_info("fio: WARNING: The fio network protocol is neither "
+			 "authenticated nor encrypted.\n");
+		log_info("fio: WARNING: Use this mode only in trusted "
+			 "environments.\n");
+	}
+
 	log_info("fio: server listening on %s\n", bind_str);
 
 	if (listen(sk, 4) < 0) {
-- 
2.55.0

Re: [PATCH 3/3] fio: server: Warn when listening on network

[Jens Axboe]

On 9/17/26 1:41 PM, Eric Sandeen wrote:
> Make it crystal clear to users at run time that the client/server
> protocol should only be used in trusted environments.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
>  server.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/server.c b/server.c
> index 34f3f5c0..887ed7ed 100644
> --- a/server.c
> +++ b/server.c
> @@ -2596,6 +2596,13 @@ static int fio_init_server_connection(void)
>  	} else
>  		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
>  
> +	if (!bind_sock) {
> +		log_info("fio: WARNING: The fio network protocol is neither "
> +			 "authenticated nor encrypted.\n");
> +		log_info("fio: WARNING: Use this mode only in trusted "
> +			 "environments.\n");
> +	}
> +
>  	log_info("fio: server listening on %s\n", bind_str);
>  
>  	if (listen(sk, 4) < 0) {

Let's not pollute the log with that - just document it in the...
documentation!

Like I said in a private email, anyone that runs a fio backend
outside of a trusted local/test environment needs to get their
head examined. By definition, since it's generally used to open
block devices and read/write to them it's run as root. And since
the environment should be trusted, so should the clients. Anyone
thinking otherwise needs to get their head examined.

-- 
Jens Axboe

Re: [PATCH 3/3] fio: server: Warn when listening on network

[Eric Sandeen]

On 9/17/26 3:01 PM, Jens Axboe wrote:
> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>> Make it crystal clear to users at run time that the client/server
>> protocol should only be used in trusted environments.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>  server.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/server.c b/server.c
>> index 34f3f5c0..887ed7ed 100644
>> --- a/server.c
>> +++ b/server.c
>> @@ -2596,6 +2596,13 @@ static int fio_init_server_connection(void)
>>  	} else
>>  		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
>>  
>> +	if (!bind_sock) {
>> +		log_info("fio: WARNING: The fio network protocol is neither "
>> +			 "authenticated nor encrypted.\n");
>> +		log_info("fio: WARNING: Use this mode only in trusted "
>> +			 "environments.\n");
>> +	}
>> +
>>  	log_info("fio: server listening on %s\n", bind_str);
>>  
>>  	if (listen(sk, 4) < 0) {
> 
> Let's not pollute the log with that - just document it in the...
> documentation!

Which is implicitly already there in the man page, so it's probably
covered. Could make it more explicit I suppose, tho that's still
assuming users will read it, heh.

> Like I said in a private email, anyone that runs a fio backend
> outside of a trusted local/test environment needs to get their
> head examined. By definition, since it's generally used to open
> block devices and read/write to them it's run as root. And since
> the environment should be trusted, so should the clients. Anyone
> thinking otherwise needs to get their head examined.

I don't disagree. Just floating the idea of making it crystal clear
at runtime, and like I said, if you don't like the change, I'm
happy to drop it.

Thanks,
-Eric

Re: [PATCH 3/3] fio: server: Warn when listening on network

[Jens Axboe]

On 9/17/26 2:29 PM, Eric Sandeen wrote:
> On 9/17/26 3:01 PM, Jens Axboe wrote:
>> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>>> Make it crystal clear to users at run time that the client/server
>>> protocol should only be used in trusted environments.
>>>
>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>> ---
>>>  server.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/server.c b/server.c
>>> index 34f3f5c0..887ed7ed 100644
>>> --- a/server.c
>>> +++ b/server.c
>>> @@ -2596,6 +2596,13 @@ static int fio_init_server_connection(void)
>>>  	} else
>>>  		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
>>>  
>>> +	if (!bind_sock) {
>>> +		log_info("fio: WARNING: The fio network protocol is neither "
>>> +			 "authenticated nor encrypted.\n");
>>> +		log_info("fio: WARNING: Use this mode only in trusted "
>>> +			 "environments.\n");
>>> +	}
>>> +
>>>  	log_info("fio: server listening on %s\n", bind_str);
>>>  
>>>  	if (listen(sk, 4) < 0) {
>>
>> Let's not pollute the log with that - just document it in the...
>> documentation!
> 
> Which is implicitly already there in the man page, so it's probably
> covered. Could make it more explicit I suppose, tho that's still
> assuming users will read it, heh.

The only people that will read this one are the ones that see it
in a log, breaking their scripts, not the intended audience.
If it's already documented in the man page, then just point any
report like that at the man page!

>> Like I said in a private email, anyone that runs a fio backend
>> outside of a trusted local/test environment needs to get their
>> head examined. By definition, since it's generally used to open
>> block devices and read/write to them it's run as root. And since
>> the environment should be trusted, so should the clients. Anyone
>> thinking otherwise needs to get their head examined.
> 
> I don't disagree. Just floating the idea of making it crystal clear
> at runtime, and like I said, if you don't like the change, I'm
> happy to drop it.

I will just pick up patch 2/3, I think that one is obviously fine.

-- 
Jens Axboe

Re: [PATCH 3/3] fio: server: Warn when listening on network

[Jens Axboe]

On 9/17/26 2:45 PM, Jens Axboe wrote:
>> I don't disagree. Just floating the idea of making it crystal clear
>> at runtime, and like I said, if you don't like the change, I'm
>> happy to drop it.
> 
> I will just pick up patch 2/3, I think that one is obviously fine.

I guess that depends on patch 1 - so please do resend patch 2, and I
will get it applied.

-- 
Jens Axboe

Re: [PATCH 3/3] fio: server: Warn when listening on network

[Eric Sandeen]

On 9/17/26 3:45 PM, Jens Axboe wrote:
> On 9/17/26 2:29 PM, Eric Sandeen wrote:
>> On 9/17/26 3:01 PM, Jens Axboe wrote:
>>> On 9/17/26 1:41 PM, Eric Sandeen wrote:
>>>> Make it crystal clear to users at run time that the client/server
>>>> protocol should only be used in trusted environments.
>>>>
>>>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>>>> ---
>>>>  server.c | 7 +++++++
>>>>  1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/server.c b/server.c
>>>> index 34f3f5c0..887ed7ed 100644
>>>> --- a/server.c
>>>> +++ b/server.c
>>>> @@ -2596,6 +2596,13 @@ static int fio_init_server_connection(void)
>>>>  	} else
>>>>  		snprintf(bind_str, sizeof(bind_str), "%s", bind_sock);
>>>>  
>>>> +	if (!bind_sock) {
>>>> +		log_info("fio: WARNING: The fio network protocol is neither "
>>>> +			 "authenticated nor encrypted.\n");
>>>> +		log_info("fio: WARNING: Use this mode only in trusted "
>>>> +			 "environments.\n");
>>>> +	}
>>>> +
>>>>  	log_info("fio: server listening on %s\n", bind_str);
>>>>  
>>>>  	if (listen(sk, 4) < 0) {
>>>
>>> Let's not pollute the log with that - just document it in the...
>>> documentation!
>>
>> Which is implicitly already there in the man page, so it's probably
>> covered. Could make it more explicit I suppose, tho that's still
>> assuming users will read it, heh.
> 
> The only people that will read this one are the ones that see it
> in a log, breaking their scripts, not the intended audience.
> If it's already documented in the man page, then just point any
> report like that at the man page!

Fair.

>>> Like I said in a private email, anyone that runs a fio backend
>>> outside of a trusted local/test environment needs to get their
>>> head examined. By definition, since it's generally used to open
>>> block devices and read/write to them it's run as root. And since
>>> the environment should be trusted, so should the clients. Anyone
>>> thinking otherwise needs to get their head examined.
>>
>> I don't disagree. Just floating the idea of making it crystal clear
>> at runtime, and like I said, if you don't like the change, I'm
>> happy to drop it.
> 
> I will just pick up patch 2/3, I think that one is obviously fine.

Sounds good, thanks Jens!

-Eric