[PATCH 0/3] 9pfs: fix illegal names with Twstat rename

[PATCH 0/3] 9pfs: fix illegal names with Twstat rename

[Christian Schoenebeck]

This series fixes a guest-triggerable assertion fault (DoS) caused by
sending an illegal new name with the legacy Twstat rename handler.

 - Patch 1: This is the core fix that prevents the DoS vulnerability.

 - Patch 2: Additionally rejects "." and ".." as new names with Twstat
   rename operations (not being a vulnerability though).

 - Patch 3: Consolidates the name validation logic spread multiple
   times over multiple request handlers.

Christian Schoenebeck (3):
  hw/9pfs: fix abort due to illegal name with Twstat rename
  hw/9pfs: reject . and .. in Twstat rename
  hw/9pfs: consolidate name validation with check_name()

 hw/9pfs/9p.c | 97 +++++++++++++++++++++++-----------------------------
 1 file changed, 42 insertions(+), 55 deletions(-)

-- 
2.47.3

[PATCH 1/3] hw/9pfs: fix abort due to illegal name with Twstat rename

[Christian Schoenebeck]

The legacy Twstat 9p request can be used to rename files and directories.
Unlike the other, more recent rename requests like Trename and Trenameat,
Twstat does not validate the submitted new name before passing it to
v9fs_complete_rename().

A priviliged guest user with direct communication access to 9p server
could pass a string containing '/' as new name, which causes an assertion
fault (DoS) in local_name_to_path().

Fix this by rejecting such strings by checking the client supplied new
name with name_is_illegal(), similar to how Trename and Trenameat handlers
do already.

Reported-by: Feifan Qian <bea1e@proton.me>
Fixes: 8cf89e007a ("virtio-9p: Add P9_TWSTAT support")
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index e2713b9eee..88fb28b318 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3617,6 +3617,11 @@ static void coroutine_fn v9fs_wstat(void *opaque)
             err = -EOPNOTSUPP;
             goto out;
         }
+        if (name_is_illegal(v9stat.name.data)) {
+            err = -ENOENT;
+            goto out;
+        }
+
         v9fs_path_write_lock(s);
         err = v9fs_complete_rename(pdu, fidp, -1, &v9stat.name);
         v9fs_path_unlock(s);
-- 
2.47.3

[PATCH 2/3] hw/9pfs: reject . and .. in Twstat rename

[Christian Schoenebeck]

The other Trename and Trenameat handlers already reject "." and ".."
as new name on rename requests by returning -EISDIR in this case.

The legacy Twstat rename handler is missing this validation. While passing
"." or ".." does not trigger a crash as fixed by the previous patch (since
the fs backend driver's system calls handle these gracefully), it creates
a behavioral inconsistency, as it is semantically meaningless to rename a
file to a directory reference in the first place.

Fix this by rejecting "." and ".." in Twstat rename handler with -EISDIR
to match behavior of Trename and Trenameat handlers.

Fixes: 8cf89e007a ("virtio-9p: Add P9_TWSTAT support")
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 88fb28b318..a09f99537d 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3621,6 +3621,10 @@ static void coroutine_fn v9fs_wstat(void *opaque)
             err = -ENOENT;
             goto out;
         }
+        if (!strcmp(".", v9stat.name.data) || !strcmp("..", v9stat.name.data)) {
+            err = -EISDIR;
+            goto out;
+        }
 
         v9fs_path_write_lock(s);
         err = v9fs_complete_rename(pdu, fidp, -1, &v9stat.name);
-- 
2.47.3

[PATCH 3/3] hw/9pfs: consolidate name validation with check_name()

[Christian Schoenebeck]

Add a new, shared helper function check_name() that consolidates the name
validation logic (illegal name check and "." / ".." rejection) currently
spread over multiple 9p handlers, unnecessarily duplicating code.

This is pure refactoring with no behavior change. The existing error code
semantics are preserved: rename operations return -EISDIR, create
operations return -EEXIST.

Note: These current error codes actually differ from native Linux system
calls (e.g. rename() returns -EBUSY, open(O_CREAT) returns -EISDIR). The 9P
protocol does not mandate specific error codes for these validation errors.
Hence consolidating to a single error code (e.g., -EINVAL) for all cases
could be considered in the future for simplicity reason.

This change reduces code duplication across 9 functions:

- v9fs_lcreate
- v9fs_create
- v9fs_symlink
- v9fs_link
- v9fs_rename
- v9fs_renameat
- v9fs_wstat
- v9fs_mknod
- v9fs_mkdir

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 100 ++++++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 61 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index a09f99537d..9b46b33ba8 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1810,6 +1810,25 @@ static bool name_is_illegal(const char *name)
     return !*name || strchr(name, '/') != NULL;
 }
 
+static int check_name(const char *name, V9fsPDU *pdu)
+{
+    int request_type = pdu->id;
+
+    if (name_is_illegal(name)) {
+        return -ENOENT;
+    }
+    if (!strcmp(name, ".") || !strcmp(name, "..")) {
+        /*
+         * TODO: The different error codes here are just there to preserve
+         * pre-existing behaviour of 9p server. In future it might make sense to
+         * consolidate this and e.g. just return -EINVAL for everyone.
+         */
+        return (request_type == P9_TRENAME || request_type == P9_TRENAMEAT ||
+                request_type == P9_TWSTAT) ? -EISDIR : -EEXIST;
+    }
+    return 0;
+}
+
 static bool same_stat_id(const struct stat *a, const struct stat *b)
 {
     return a->st_dev == b->st_dev && a->st_ino == b->st_ino;
@@ -2160,13 +2179,8 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
     }
     trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -2848,13 +2862,8 @@ static void coroutine_fn v9fs_create(void *opaque)
     }
     trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -3042,13 +3051,8 @@ static void coroutine_fn v9fs_symlink(void *opaque)
     }
     trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -3135,13 +3139,8 @@ static void coroutine_fn v9fs_link(void *opaque)
     }
     trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -3367,13 +3366,8 @@ static void coroutine_fn v9fs_rename(void *opaque)
         goto out_nofid;
     }
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EISDIR;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -3505,14 +3499,12 @@ static void coroutine_fn v9fs_renameat(void *opaque)
         goto out_err;
     }
 
-    if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) {
-        err = -ENOENT;
+    err = check_name(old_name.data, pdu);
+    if (err < 0) {
         goto out_err;
     }
-
-    if (!strcmp(".", old_name.data) || !strcmp("..", old_name.data) ||
-        !strcmp(".", new_name.data) || !strcmp("..", new_name.data)) {
-        err = -EISDIR;
+    err = check_name(new_name.data, pdu);
+    if (err < 0) {
         goto out_err;
     }
 
@@ -3617,12 +3609,8 @@ static void coroutine_fn v9fs_wstat(void *opaque)
             err = -EOPNOTSUPP;
             goto out;
         }
-        if (name_is_illegal(v9stat.name.data)) {
-            err = -ENOENT;
-            goto out;
-        }
-        if (!strcmp(".", v9stat.name.data) || !strcmp("..", v9stat.name.data)) {
-            err = -EISDIR;
+        err = check_name(v9stat.name.data, pdu);
+        if (err < 0) {
             goto out;
         }
 
@@ -3755,13 +3743,8 @@ static void coroutine_fn v9fs_mknod(void *opaque)
     }
     trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
@@ -3917,13 +3900,8 @@ static void coroutine_fn v9fs_mkdir(void *opaque)
     }
     trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid);
 
-    if (name_is_illegal(name.data)) {
-        err = -ENOENT;
-        goto out_nofid;
-    }
-
-    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
-        err = -EEXIST;
+    err = check_name(name.data, pdu);
+    if (err < 0) {
         goto out_nofid;
     }
 
-- 
2.47.3

Re: [PATCH 0/3] 9pfs: fix illegal names with Twstat rename

[Christian Schoenebeck]

On Friday, 29 May 2026 18:30:38 CEST Christian Schoenebeck wrote:
> This series fixes a guest-triggerable assertion fault (DoS) caused by
> sending an illegal new name with the legacy Twstat rename handler.
> 
>  - Patch 1: This is the core fix that prevents the DoS vulnerability.
> 
>  - Patch 2: Additionally rejects "." and ".." as new names with Twstat
>    rename operations (not being a vulnerability though).
> 
>  - Patch 3: Consolidates the name validation logic spread multiple
>    times over multiple request handlers.
> 
> Christian Schoenebeck (3):
>   hw/9pfs: fix abort due to illegal name with Twstat rename
>   hw/9pfs: reject . and .. in Twstat rename
>   hw/9pfs: consolidate name validation with check_name()
> 
>  hw/9pfs/9p.c | 97 +++++++++++++++++++++++-----------------------------
>  1 file changed, 42 insertions(+), 55 deletions(-)

Queued on 9p.next:
https://github.com/cschoenebeck/qemu/commits/9p.next

Thanks!

/Christian