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

["Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>]

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