diff mbox

[v4,2/3] 9pfs: forbid . and .. in file names

Message ID 147257707176.28515.1734765316772223928.stgit@bahia.lab.toulouse-stg.fr.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Greg Kurz Aug. 30, 2016, 5:13 p.m. UTC
According to the 9P spec http://man.cat-v.org/plan_9/5/open about the
create request:

The names . and .. are special; it is illegal to create files with these
names.

This patch causes the create and lcreate requests to fail with EINVAL if
the file name is either "." or "..".

Even if it isn't explicitly written in the spec, this patch extends the
checking to all requests that may cause a directory entry to be created:

    - mknod
    - rename
    - renameat
    - mkdir
    - link
    - symlink

The unlinkat request also gets patched for consistency (even if
rmdir("foo/..") is expected to fail according to POSIX.1-2001).

The various error values come from the linux manual pages.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Greg Kurz <groug@kaod.org>

v3: - rename and renameat now return EISDIR instead of EBUSY
---
 hw/9pfs/9p.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

Comments

Eric Blake Aug. 30, 2016, 6:06 p.m. UTC | #1
On 08/30/2016 12:13 PM, Greg Kurz wrote:
> According to the 9P spec http://man.cat-v.org/plan_9/5/open about the
> create request:
> 
> The names . and .. are special; it is illegal to create files with these
> names.
> 
> This patch causes the create and lcreate requests to fail with EINVAL if
> the file name is either "." or "..".
> 
> Even if it isn't explicitly written in the spec, this patch extends the
> checking to all requests that may cause a directory entry to be created:
> 
>     - mknod
>     - rename
>     - renameat
>     - mkdir
>     - link
>     - symlink
> 
> The unlinkat request also gets patched for consistency (even if
> rmdir("foo/..") is expected to fail according to POSIX.1-2001).
> 
> The various error values come from the linux manual pages.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> 
> v3: - rename and renameat now return EISDIR instead of EBUSY

The v3 comment could occur after the '---' separator.

> ---
>  hw/9pfs/9p.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 51 insertions(+)

Maintainer can touch that up, then add
Reviewed-by: Eric Blake <eblake@redhat.com>
Greg Kurz Aug. 30, 2016, 7:03 p.m. UTC | #2
On Tue, 30 Aug 2016 13:06:31 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 08/30/2016 12:13 PM, Greg Kurz wrote:
> > According to the 9P spec http://man.cat-v.org/plan_9/5/open about the
> > create request:
> > 
> > The names . and .. are special; it is illegal to create files with these
> > names.
> > 
> > This patch causes the create and lcreate requests to fail with EINVAL if
> > the file name is either "." or "..".
> > 
> > Even if it isn't explicitly written in the spec, this patch extends the
> > checking to all requests that may cause a directory entry to be created:
> > 
> >     - mknod
> >     - rename
> >     - renameat
> >     - mkdir
> >     - link
> >     - symlink
> > 
> > The unlinkat request also gets patched for consistency (even if
> > rmdir("foo/..") is expected to fail according to POSIX.1-2001).
> > 
> > The various error values come from the linux manual pages.
> > 
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > 
> > v3: - rename and renameat now return EISDIR instead of EBUSY  
> 
> The v3 comment could occur after the '---' separator.
> 

Yes of course. Sorry for the other patches as well :)

> > ---
> >  hw/9pfs/9p.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 51 insertions(+)  
> 
> Maintainer can touch that up, then add
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
diff mbox

Patch

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 385269ea0ac3..51c6f9883bf8 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1497,6 +1497,11 @@  static void v9fs_lcreate(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     fidp = get_fid(pdu, dfid);
     if (fidp == NULL) {
         err = -ENOENT;
@@ -2096,6 +2101,11 @@  static void v9fs_create(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         err = -EINVAL;
@@ -2266,6 +2276,11 @@  static void v9fs_symlink(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     dfidp = get_fid(pdu, dfid);
     if (dfidp == NULL) {
         err = -EINVAL;
@@ -2345,6 +2360,11 @@  static void v9fs_link(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     dfidp = get_fid(pdu, dfid);
     if (dfidp == NULL) {
         err = -ENOENT;
@@ -2433,6 +2453,16 @@  static void v9fs_unlinkat(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data)) {
+        err = -EINVAL;
+        goto out_nofid;
+    }
+
+    if (!strcmp("..", name.data)) {
+        err = -ENOTEMPTY;
+        goto out_nofid;
+    }
+
     dfidp = get_fid(pdu, dfid);
     if (dfidp == NULL) {
         err = -EINVAL;
@@ -2545,6 +2575,11 @@  static void v9fs_rename(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EISDIR;
+        goto out_nofid;
+    }
+
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         err = -ENOENT;
@@ -2662,6 +2697,12 @@  static void v9fs_renameat(void *opaque)
         goto out_err;
     }
 
+    if (!strcmp(".", old_name.data) || !strcmp("..", old_name.data) ||
+        !strcmp(".", new_name.data) || !strcmp("..", new_name.data)) {
+        err = -EISDIR;
+        goto out_err;
+    }
+
     v9fs_path_write_lock(s);
     err = v9fs_complete_renameat(pdu, olddirfid,
                                  &old_name, newdirfid, &new_name);
@@ -2877,6 +2918,11 @@  static void v9fs_mknod(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         err = -ENOENT;
@@ -3033,6 +3079,11 @@  static void v9fs_mkdir(void *opaque)
         goto out_nofid;
     }
 
+    if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
+        err = -EEXIST;
+        goto out_nofid;
+    }
+
     fidp = get_fid(pdu, fid);
     if (fidp == NULL) {
         err = -ENOENT;