* [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths
@ 2026-10-14 19:38 Richard Purdie
2026-10-15 13:38 ` [yocto-patches] " Mathieu Dubois-Briand
0 siblings, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2026-10-14 19:38 UTC (permalink / raw)
To: yocto-patches; +Cc: seebs, mark.hatle
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;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths
[not found] <18DE7C971DAD1AD4.2478941@lists.yoctoproject.org>
@ 2026-10-15 11:44 ` Richard Purdie
0 siblings, 0 replies; 5+ messages in thread
From: Richard Purdie @ 2026-10-15 11:44 UTC (permalink / raw)
To: yocto-patches; +Cc: seebs, mark.hatle
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths
2026-10-14 19:38 [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths Richard Purdie
@ 2026-10-15 13:38 ` Mathieu Dubois-Briand
2026-10-15 14:25 ` Richard Purdie
0 siblings, 1 reply; 5+ messages in thread
From: Mathieu Dubois-Briand @ 2026-10-15 13:38 UTC (permalink / raw)
To: yocto-patches; +Cc: seebs, mark.hatle
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths
2026-10-15 13:38 ` [yocto-patches] " Mathieu Dubois-Briand
@ 2026-10-15 14:25 ` Richard Purdie
2026-10-15 15:43 ` Mathieu Dubois-Briand
0 siblings, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2026-10-15 14:25 UTC (permalink / raw)
To: yocto-patches; +Cc: seebs, mark.hatle
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [yocto-patches] [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths
2026-10-15 14:25 ` Richard Purdie
@ 2026-10-15 15:43 ` Mathieu Dubois-Briand
0 siblings, 0 replies; 5+ messages in thread
From: Mathieu Dubois-Briand @ 2026-10-15 15:43 UTC (permalink / raw)
To: yocto-patches; +Cc: seebs, mark.hatle
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-15 15:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-14 19:38 [pseudo] [PATCH] pseudo_client: Fix xattr/rename handling with empty paths Richard Purdie
2026-10-15 13:38 ` [yocto-patches] " Mathieu Dubois-Briand
2026-10-15 14:25 ` Richard Purdie
2026-10-15 15:43 ` Mathieu Dubois-Briand
[not found] <18DE7C971DAD1AD4.2478941@lists.yoctoproject.org>
2026-10-15 11:44 ` Richard Purdie
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.