Commit fff39a7a authored by Greg Kurz's avatar Greg Kurz Committed by Peter Maydell
Browse files

9pfs: forbid illegal path names 

Empty path components don't make sense for most commands and may cause
undefined behavior, depending on the backend.

Also, the walk request described in the 9P spec [1] clearly shows that
the client is supposed to send individual path components: the official
linux client never sends portions of path containing the / character for
example.

Moreover, the 9P spec [2] also states that a system can decide to restrict
the set of supported characters used in path components, with an explicit
mention "to remove slashes from name components".

This patch introduces a new name_is_illegal() helper that checks the
names sent by the client are not empty and don't contain unwanted chars.
Since 9pfs is only supported on linux hosts, only the / character is
checked at the moment. When support for other hosts (AKA. win32) is added,
other chars may need to be blacklisted as well.

If a client sends an illegal path component, the request will fail and
ENOENT is returned to the client.

[1] http://man.cat-v.org/plan_9/5/walk
[2] http://man.cat-v.org/plan_9/5/intro



Suggested-by: default avatarPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: default avatarGreg Kurz <groug@kaod.org>
Reviewed-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarMichael S. Tsirkin <mst@redhat.com>
Signed-off-by: default avatarPeter Maydell <peter.maydell@linaro.org>
parent 2b294f6b

hw/9pfs/9p.c

+56 −0
Original line number Diff line number Diff line
@@ -1256,6 +1256,11 @@ static int v9fs_walk_marshal(V9fsPDU *pdu, uint16_t nwnames, V9fsQID *qids) 
    return offset;

}



static bool name_is_illegal(const char *name)

{

    return !*name || strchr(name, '/') != NULL;

}



static void v9fs_walk(void *opaque)

{

    int name_idx;
@@ -1289,6 +1294,10 @@ static void v9fs_walk(void *opaque) 
            if (err < 0) {

                goto out_nofid;

            }

            if (name_is_illegal(wnames[i].data)) {

                err = -ENOENT;

                goto out_nofid;

            }

            offset += err;

        }

    } else if (nwnames > P9_MAXWELEM) {
@@ -1483,6 +1492,11 @@ static void 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;

    }



    fidp = get_fid(pdu, dfid);

    if (fidp == NULL) {

        err = -ENOENT;
@@ -2077,6 +2091,11 @@ static void 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;

    }



    fidp = get_fid(pdu, fid);

    if (fidp == NULL) {

        err = -EINVAL;
@@ -2242,6 +2261,11 @@ static void 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;

    }



    dfidp = get_fid(pdu, dfid);

    if (dfidp == NULL) {

        err = -EINVAL;
@@ -2316,6 +2340,11 @@ static void 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;

    }



    dfidp = get_fid(pdu, dfid);

    if (dfidp == NULL) {

        err = -ENOENT;
@@ -2398,6 +2427,12 @@ static void v9fs_unlinkat(void *opaque) 
    if (err < 0) {

        goto out_nofid;

    }



    if (name_is_illegal(name.data)) {

        err = -ENOENT;

        goto out_nofid;

    }



    dfidp = get_fid(pdu, dfid);

    if (dfidp == NULL) {

        err = -EINVAL;
@@ -2504,6 +2539,12 @@ static void v9fs_rename(void *opaque) 
    if (err < 0) {

        goto out_nofid;

    }



    if (name_is_illegal(name.data)) {

        err = -ENOENT;

        goto out_nofid;

    }



    fidp = get_fid(pdu, fid);

    if (fidp == NULL) {

        err = -ENOENT;
@@ -2616,6 +2657,11 @@ static void v9fs_renameat(void *opaque) 
        goto out_err;

    }



    if (name_is_illegal(old_name.data) || name_is_illegal(new_name.data)) {

        err = -ENOENT;

        goto out_err;

    }



    v9fs_path_write_lock(s);

    err = v9fs_complete_renameat(pdu, olddirfid,

                                 &old_name, newdirfid, &new_name);
@@ -2826,6 +2872,11 @@ static void 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;

    }



    fidp = get_fid(pdu, fid);

    if (fidp == NULL) {

        err = -ENOENT;
@@ -2977,6 +3028,11 @@ static void 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;

    }



    fidp = get_fid(pdu, fid);

    if (fidp == NULL) {

        err = -ENOENT;