[pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Richard Purdie]

On Fedora45, we can end up in a situation where path_extra_1 (for xattr/rename ops)
but path is of zero length. The path_extra_1 codepath does not handle this well and
corrupts memory.

The issue is "partial_len = pathlen - 1 - strip_slash" when pathlen is 0 (and
hence strip_slash is also 0). This leads to invalid memory writes and if those
happen to access off the end of a mapped segment, the program segfaults.

Fix the code to handle the pathlen=0 case.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 pseudo_client.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/pseudo_client.c b/pseudo_client.c
index 23d7b0f..002de30 100644
--- a/pseudo_client.c
+++ b/pseudo_client.c
@@ -1789,7 +1789,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 	 */
 	if (path_extra_1) {
 		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
-		size_t partial_len = pathlen - 1 - strip_slash;
+		size_t partial_len = pathlen - strip_slash;
 		if (path_extra_2) {
 			full_len += path_extra_2len + 1;
 		}
@@ -1804,12 +1804,17 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
 				return 0;
 			}
 		}
-		memcpy(alloced_path, path, partial_len);
-		alloced_path[partial_len] = '\0';
-		memcpy(alloced_path + partial_len + 1, path_extra_1, path_extra_1len);
-		alloced_path[partial_len + path_extra_1len + 1] = '\0';
+		if (pathlen) {
+			memcpy(alloced_path, path, partial_len - 1);
+			alloced_path[partial_len - 1] = '\0';
+		} else {
+			alloced_path[0] = '\0';
+			partial_len = 1;
+		}
+		memcpy(alloced_path + partial_len, path_extra_1, path_extra_1len);
+		alloced_path[partial_len + path_extra_1len] = '\0';
 		if (path_extra_2) {
-			memcpy(alloced_path + partial_len + path_extra_1len + 2, path_extra_2, path_extra_2len);
+			memcpy(alloced_path + partial_len + path_extra_1len + 1, path_extra_2, path_extra_2len);
 		}
 		alloced_path[full_len - 1] = '\0';
 		path = alloced_path;

Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Mathieu Dubois-Briand]

On Wed Oct 14, 2026 at 9:38 PM CEST, Richard Purdie via lists.yoctoproject.org wrote:
> On Fedora45, we can end up in a situation where path_extra_1 (for xattr/rename ops)
> but path is of zero length. The path_extra_1 codepath does not handle this well and
> corrupts memory.
>
> The issue is "partial_len = pathlen - 1 - strip_slash" when pathlen is 0 (and
> hence strip_slash is also 0). This leads to invalid memory writes and if those
> happen to access off the end of a mapped segment, the program segfaults.
>
> Fix the code to handle the pathlen=0 case.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  pseudo_client.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/pseudo_client.c b/pseudo_client.c
> index 23d7b0f..002de30 100644
> --- a/pseudo_client.c
> +++ b/pseudo_client.c
> @@ -1789,7 +1789,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
>  	 */
>  	if (path_extra_1) {
>  		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
> -		size_t partial_len = pathlen - 1 - strip_slash;
> +		size_t partial_len = pathlen - strip_slash;
>  		if (path_extra_2) {
>  			full_len += path_extra_2len + 1;
>  		}
> @@ -1804,12 +1804,17 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
>  				return 0;
>  			}
>  		}
> -		memcpy(alloced_path, path, partial_len);
> -		alloced_path[partial_len] = '\0';
> -		memcpy(alloced_path + partial_len + 1, path_extra_1, path_extra_1len);
> -		alloced_path[partial_len + path_extra_1len + 1] = '\0';
> +		if (pathlen) {
> +			memcpy(alloced_path, path, partial_len - 1);
> +			alloced_path[partial_len - 1] = '\0';
> +		} else {
> +			alloced_path[0] = '\0';
> +			partial_len = 1;

Err, I have a doubt here.

Setting partial_len to 1 looks correct, but it was previously used to
compute full_len, that is used to check the if buffer is big enough
(full_len > alloced_len).

So if pathlen is 0, the buffer can be 1 byte too short, right? Just
incrementing partial_len a bit before should fix that.

Thanks,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Richard Purdie]

On Thu, 2026-10-15 at 15:38 +0200, Mathieu Dubois-Briand via lists.yoctoproject.org wrote:
> On Wed Oct 14, 2026 at 9:38 PM CEST, Richard Purdie via lists.yoctoproject.org wrote:
> > On Fedora45, we can end up in a situation where path_extra_1 (for xattr/rename ops)
> > but path is of zero length. The path_extra_1 codepath does not handle this well and
> > corrupts memory.
> > 
> > The issue is "partial_len = pathlen - 1 - strip_slash" when pathlen is 0 (and
> > hence strip_slash is also 0). This leads to invalid memory writes and if those
> > happen to access off the end of a mapped segment, the program segfaults.
> > 
> > Fix the code to handle the pathlen=0 case.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  pseudo_client.c | 17 +++++++++++------
> >  1 file changed, 11 insertions(+), 6 deletions(-)
> > 
> > diff --git a/pseudo_client.c b/pseudo_client.c
> > index 23d7b0f..002de30 100644
> > --- a/pseudo_client.c
> > +++ b/pseudo_client.c
> > @@ -1789,7 +1789,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
> >  	 */
> >  	if (path_extra_1) {
> >  		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
> > -		size_t partial_len = pathlen - 1 - strip_slash;
> > +		size_t partial_len = pathlen - strip_slash;
> >  		if (path_extra_2) {
> >  			full_len += path_extra_2len + 1;
> >  		}
> > @@ -1804,12 +1804,17 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
> >  				return 0;
> >  			}
> >  		}
> > -		memcpy(alloced_path, path, partial_len);
> > -		alloced_path[partial_len] = '\0';
> > -		memcpy(alloced_path + partial_len + 1, path_extra_1, path_extra_1len);
> > -		alloced_path[partial_len + path_extra_1len + 1] = '\0';
> > +		if (pathlen) {
> > +			memcpy(alloced_path, path, partial_len - 1);
> > +			alloced_path[partial_len - 1] = '\0';
> > +		} else {
> > +			alloced_path[0] = '\0';
> > +			partial_len = 1;
> 
> Err, I have a doubt here.
> 
> Setting partial_len to 1 looks correct, but it was previously used to
> compute full_len, that is used to check the if buffer is big enough
> (full_len > alloced_len).
> 
> So if pathlen is 0, the buffer can be 1 byte too short, right? Just
> incrementing partial_len a bit before should fix that.

I can't see where partial_len is used in the computation for full_len ?

		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
		size_t partial_len = pathlen - strip_slash;
		if (path_extra_2) {
			full_len += path_extra_2len + 1;
		}
		if (full_len > alloced_len) {
                    <realloc code>
		}

Cheers,

Richard

Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Mathieu Dubois-Briand]

On Thu Oct 15, 2026 at 4:25 PM CEST, Richard Purdie via lists.yoctoproject.org wrote:
> On Thu, 2026-10-15 at 15:38 +0200, Mathieu Dubois-Briand via lists.yoctoproject.org wrote:
>> On Wed Oct 14, 2026 at 9:38 PM CEST, Richard Purdie via lists.yoctoproject.org wrote:
>> > On Fedora45, we can end up in a situation where path_extra_1 (for xattr/rename ops)
>> > but path is of zero length. The path_extra_1 codepath does not handle this well and
>> > corrupts memory.
>> > 
>> > The issue is "partial_len = pathlen - 1 - strip_slash" when pathlen is 0 (and
>> > hence strip_slash is also 0). This leads to invalid memory writes and if those
>> > happen to access off the end of a mapped segment, the program segfaults.
>> > 
>> > Fix the code to handle the pathlen=0 case.
>> > 
>> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> > ---
>> >  pseudo_client.c | 17 +++++++++++------
>> >  1 file changed, 11 insertions(+), 6 deletions(-)
>> > 
>> > diff --git a/pseudo_client.c b/pseudo_client.c
>> > index 23d7b0f..002de30 100644
>> > --- a/pseudo_client.c
>> > +++ b/pseudo_client.c
>> > @@ -1789,7 +1789,7 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
>> >  	 */
>> >  	if (path_extra_1) {
>> >  		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
>> > -		size_t partial_len = pathlen - 1 - strip_slash;
>> > +		size_t partial_len = pathlen - strip_slash;
>> >  		if (path_extra_2) {
>> >  			full_len += path_extra_2len + 1;
>> >  		}
>> > @@ -1804,12 +1804,17 @@ pseudo_client_op(pseudo_op_t op, int access, int fd, int dirfd, const char *path
>> >  				return 0;
>> >  			}
>> >  		}
>> > -		memcpy(alloced_path, path, partial_len);
>> > -		alloced_path[partial_len] = '\0';
>> > -		memcpy(alloced_path + partial_len + 1, path_extra_1, path_extra_1len);
>> > -		alloced_path[partial_len + path_extra_1len + 1] = '\0';
>> > +		if (pathlen) {
>> > +			memcpy(alloced_path, path, partial_len - 1);
>> > +			alloced_path[partial_len - 1] = '\0';
>> > +		} else {
>> > +			alloced_path[0] = '\0';
>> > +			partial_len = 1;
>> 
>> Err, I have a doubt here.
>> 
>> Setting partial_len to 1 looks correct, but it was previously used to
>> compute full_len, that is used to check the if buffer is big enough
>> (full_len > alloced_len).
>> 
>> So if pathlen is 0, the buffer can be 1 byte too short, right? Just
>> incrementing partial_len a bit before should fix that.
>
> I can't see where partial_len is used in the computation for full_len ?
>
> 		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
> 		size_t partial_len = pathlen - strip_slash;
> 		if (path_extra_2) {
> 			full_len += path_extra_2len + 1;
> 		}
> 		if (full_len > alloced_len) {
>                     <realloc code>
> 		}
>
> Cheers,
>
> Richard
>
Yes, sorry, my previous mail does not make any sense. Let's try that
again.

So my point is, if pathlen is 0, we have:

>		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;

So full_len is going to be:
path_extra_1len + 1 + 0 (pathlen) - strip_slash
So it will be at most:
path_extra_1len + 1

>		size_t partial_len = pathlen - strip_slash;
>		if (path_extra_2) {
>			full_len += path_extra_2len + 1;
>		}
>		if (full_len > alloced_len) {
>			free(alloced_path);
>			alloced_path = malloc(full_len);
>			alloced_len = full_len;
>			if (!alloced_path) {
>				pseudo_diag("Can't allocate space for paths for a rename operation.  Sorry.\n");
>				alloced_len = 0;
>				pseudo_magic();
>				return 0;
>			}

So buffer size is at most `path_extra_1len + 1`

>		}
>		if (pathlen) {
>			memcpy(alloced_path, path, partial_len - 1);
>			alloced_path[partial_len - 1] = '\0';
>		} else {
>			alloced_path[0] = '\0';
>			partial_len = 1;

OK, partial_len is 1

>		}
>		memcpy(alloced_path + partial_len, path_extra_1, path_extra_1len);
>		alloced_path[partial_len + path_extra_1len] = '\0';

Then here we write at offset `1 + path_extra_1len`.
So buffer size should be at least `1 + path_extra_1len + 1`

>		if (path_extra_2) {
>			memcpy(alloced_path + partial_len + path_extra_1len + 1, path_extra_2, path_extra_2len);
>		}
>		alloced_path[full_len - 1] = '\0';
>		path = alloced_path;
>		pathlen = full_len;
>		pseudo_debug_call(PDBGF_IPC | PDBGF_VERBOSE, pseudo_dump_data, "combined path buffer", path, pathlen);

Hopefully this makes a bit more sense now :)

Thanks,
Mathieu

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Richard Purdie]

On Thu, 2026-10-15 at 17:43 +0200, Mathieu Dubois-Briand via lists.yoctoproject.org wrote:
> So my point is, if pathlen is 0, we have:
> 
> > 		size_t full_len = path_extra_1len + 1 + pathlen - strip_slash;
> 
> So full_len is going to be:
> path_extra_1len + 1 + 0 (pathlen) - strip_slash
> So it will be at most:
> path_extra_1len + 1
> 
> > 		size_t partial_len = pathlen - strip_slash;
> > 		if (path_extra_2) {
> > 			full_len += path_extra_2len + 1;
> > 		}
> > 		if (full_len > alloced_len) {
> > 			free(alloced_path);
> > 			alloced_path = malloc(full_len);
> > 			alloced_len = full_len;
> > 			if (!alloced_path) {
> > 				pseudo_diag("Can't allocate space for paths for a rename operation.  Sorry.\n");
> > 				alloced_len = 0;
> > 				pseudo_magic();
> > 				return 0;
> > 			}
> 
> So buffer size is at most `path_extra_1len + 1`
> 
> > 		}
> > 		if (pathlen) {
> > 			memcpy(alloced_path, path, partial_len - 1);
> > 			alloced_path[partial_len - 1] = '\0';
> > 		} else {
> > 			alloced_path[0] = '\0';
> > 			partial_len = 1;
> 
> OK, partial_len is 1
> 
> > 		}
> > 		memcpy(alloced_path + partial_len, path_extra_1, path_extra_1len);
> > 		alloced_path[partial_len + path_extra_1len] = '\0';
> 
> Then here we write at offset `1 + path_extra_1len`.
> So buffer size should be at least `1 + path_extra_1len + 1`
> 
> > 		if (path_extra_2) {
> > 			memcpy(alloced_path + partial_len + path_extra_1len + 1, path_extra_2, path_extra_2len);
> > 		}
> > 		alloced_path[full_len - 1] = '\0';
> > 		path = alloced_path;
> > 		pathlen = full_len;
> > 		pseudo_debug_call(PDBGF_IPC | PDBGF_VERBOSE, pseudo_dump_data, "combined path buffer", path, pathlen);
> 
> Hopefully this makes a bit more sense now :)

Yes, I think you're right. I'll add an extra 1 unconditionally to full_len to cover all the options...

Thanks,

Richard

Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths

[Richard Purdie]

On Wed, 2026-10-14 at 20:38 +0100, Richard Purdie via lists.yoctoproject.org wrote:
> On Fedora45, we can end up in a situation where path_extra_1 (for xattr/rename ops)
> but path is of zero length. The path_extra_1 codepath does not handle this well and
> corrupts memory.
> 
> The issue is "partial_len = pathlen - 1 - strip_slash" when pathlen is 0 (and
> hence strip_slash is also 0). This leads to invalid memory writes and if those
> happen to access off the end of a mapped segment, the program segfaults.
> 
> Fix the code to handle the pathlen=0 case.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  pseudo_client.c | 17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)

I had wondered why we had something with a file descriptor which has no
path, I was worried we were not tracking some kind of fd creation we
should be.

The answer is it is a unix domain socket. It therefore does have a path
we could track but we'd not be able to do much with it anyway so it
doesn't really matter.

The reproducer is therefore calling fgetxattr() on a domain socket
which glibc does for nameserver type operations.

Cheers,

Richard