@samitouri / QOSamiQemu / commits / 116db2986b

hw/9pfs: consolidate name validation with check_name()

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 Link: https://lore.kernel.org/qemu-devel/0573103880129eb543f07b68c77e86f2f572f6bf.1780072238.git.qemu_oss@crudebyte.com Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

Christian Schoenebeck committed May 29, 2026 at 18:29 UTC 116db2986b11c914217bbd1547815b6c7efb944a
1 file changed +39 -61
hw/9pfs/9p.c
+39 -61
@@ -1823,6 +1823,25 @@ static bool name_is_illegal(const char *name)
1823 return !*name || strchr(name, '/') != NULL;
1824 }
1825
1826 +static int check_name(const char *name, V9fsPDU *pdu)
1827 +{
1828 + int request_type = pdu->id;
1829 +
1830 + if (name_is_illegal(name)) {
1831 + return -ENOENT;
1832 + }
1833 + if (!strcmp(name, ".") || !strcmp(name, "..")) {
1834 + /*
1835 + * TODO: The different error codes here are just there to preserve
1836 + * pre-existing behaviour of 9p server. In future it might make sense to
1837 + * consolidate this and e.g. just return -EINVAL for everyone.
1838 + */
1839 + return (request_type == P9_TRENAME || request_type == P9_TRENAMEAT ||
1840 + request_type == P9_TWSTAT) ? -EISDIR : -EEXIST;
1841 + }
1842 + return 0;
1843 +}
1844 +
1845 static bool same_stat_id(const struct stat *a, const struct stat *b)
1846 {
1847 return a->st_dev == b->st_dev && a->st_ino == b->st_ino;
@@ -2173,13 +2192,8 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
2192 }
2193 trace_v9fs_lcreate(pdu->tag, pdu->id, dfid, flags, mode, gid);
2194
2176 - if (name_is_illegal(name.data)) {
2177 - err = -ENOENT;
2178 - goto out_nofid;
2179 - }
2180 -
2181 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
2182 - err = -EEXIST;
2195 + err = check_name(name.data, pdu);
2196 + if (err < 0) {
2197 goto out_nofid;
2198 }
2199
@@ -2861,13 +2875,8 @@ static void coroutine_fn v9fs_create(void *opaque)
2875 }
2876 trace_v9fs_create(pdu->tag, pdu->id, fid, name.data, perm, mode);
2877
2864 - if (name_is_illegal(name.data)) {
2865 - err = -ENOENT;
2866 - goto out_nofid;
2867 - }
2868 -
2869 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
2870 - err = -EEXIST;
2878 + err = check_name(name.data, pdu);
2879 + if (err < 0) {
2880 goto out_nofid;
2881 }
2882
@@ -3055,13 +3064,8 @@ static void coroutine_fn v9fs_symlink(void *opaque)
3064 }
3065 trace_v9fs_symlink(pdu->tag, pdu->id, dfid, name.data, symname.data, gid);
3066
3058 - if (name_is_illegal(name.data)) {
3059 - err = -ENOENT;
3060 - goto out_nofid;
3061 - }
3062 -
3063 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
3064 - err = -EEXIST;
3067 + err = check_name(name.data, pdu);
3068 + if (err < 0) {
3069 goto out_nofid;
3070 }
3071
@@ -3148,13 +3152,8 @@ static void coroutine_fn v9fs_link(void *opaque)
3152 }
3153 trace_v9fs_link(pdu->tag, pdu->id, dfid, oldfid, name.data);
3154
3151 - if (name_is_illegal(name.data)) {
3152 - err = -ENOENT;
3153 - goto out_nofid;
3154 - }
3155 -
3156 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
3157 - err = -EEXIST;
3155 + err = check_name(name.data, pdu);
3156 + if (err < 0) {
3157 goto out_nofid;
3158 }
3159
@@ -3385,13 +3384,8 @@ static void coroutine_fn v9fs_rename(void *opaque)
3384 goto out_nofid;
3385 }
3386
3388 - if (name_is_illegal(name.data)) {
3389 - err = -ENOENT;
3390 - goto out_nofid;
3391 - }
3392 -
3393 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
3394 - err = -EISDIR;
3387 + err = check_name(name.data, pdu);
3388 + if (err < 0) {
3389 goto out_nofid;
3390 }
3391
@@ -3526,14 +3520,12 @@ static void coroutine_fn v9fs_renameat(void *opaque)
3520 goto out_err;
3521 }
3522
3529 - if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) {
3530 - err = -ENOENT;
3523 + err = check_name(old_name.data, pdu);
3524 + if (err < 0) {
3525 goto out_err;
3526 }
3533 -
3534 - if (!strcmp(".", old_name.data) || !strcmp("..", old_name.data) ||
3535 - !strcmp(".", new_name.data) || !strcmp("..", new_name.data)) {
3536 - err = -EISDIR;
3527 + err = check_name(new_name.data, pdu);
3528 + if (err < 0) {
3529 goto out_err;
3530 }
3531
@@ -3638,12 +3630,8 @@ static void coroutine_fn v9fs_wstat(void *opaque)
3630 err = -EOPNOTSUPP;
3631 goto out;
3632 }
3641 - if (name_is_illegal(v9stat.name.data)) {
3642 - err = -ENOENT;
3643 - goto out;
3644 - }
3645 - if (!strcmp(".", v9stat.name.data) || !strcmp("..", v9stat.name.data)) {
3646 - err = -EISDIR;
3633 + err = check_name(v9stat.name.data, pdu);
3634 + if (err < 0) {
3635 goto out;
3636 }
3637
@@ -3776,13 +3764,8 @@ static void coroutine_fn v9fs_mknod(void *opaque)
3764 }
3765 trace_v9fs_mknod(pdu->tag, pdu->id, fid, mode, major, minor);
3766
3779 - if (name_is_illegal(name.data)) {
3780 - err = -ENOENT;
3781 - goto out_nofid;
3782 - }
3783 -
3784 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
3785 - err = -EEXIST;
3767 + err = check_name(name.data, pdu);
3768 + if (err < 0) {
3769 goto out_nofid;
3770 }
3771
@@ -3938,13 +3921,8 @@ static void coroutine_fn v9fs_mkdir(void *opaque)
3921 }
3922 trace_v9fs_mkdir(pdu->tag, pdu->id, fid, name.data, mode, gid);
3923
3941 - if (name_is_illegal(name.data)) {
3942 - err = -ENOENT;
3943 - goto out_nofid;
3944 - }
3945 -
3946 - if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
3947 - err = -EEXIST;
3924 + err = check_name(name.data, pdu);
3925 + if (err < 0) {
3926 goto out_nofid;
3927 }
3928