[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 3/5] 9p: forbid . and .. in file names
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH v2 3/5] 9p: forbid . and .. in file names |
Date: |
Sun, 28 Aug 2016 16:06:00 +0200 |
On Fri, 26 Aug 2016 13:49:00 -0500
Eric Blake <address@hidden> wrote:
> On 08/26/2016 10:07 AM, 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 filename 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.
>
> Linux doesn't always obey the POSIX rules for which errno to use, but I
> think your choices here are mostly okay.
>
> >
> > Suggested-by: Peter Maydell <address@hidden>
> > Signed-off-by: Greg Kurz <address@hidden>
> > ---
> > hw/9pfs/9p.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > @@ -2545,6 +2575,11 @@ static void v9fs_rename(void *opaque)
> > goto out_nofid;
> > }
> >
> > + if (!strcmp(".", name.data) || !strcmp("..", name.data)) {
> > + err = -EBUSY;
> > + goto out_nofid;
> > + }
>
> POSIX suggests that EISDIR is better than EBUSY here.
>
Ok.
> > +
> > 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 = -EBUSY;
>
> Ditto.
>
Ok.
> Wait. Why is v9fs_rename() only checking one name, but v9fs_renameat()
> checking both old_name and new_name? Also, should link be checking both
> the source and destination name?
>
v9fs_rename() and v9fs_link() only take a single name argument (see the "dds"
mask passed to pdu_unmarshal()).
> > @@ -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;
> > + }
> > +
>
> Unrelated to this patch, but why do we have v9fs_renameat but not
> v9fs_mkdirat?
>
I don't know but I guess it is simply not implemented.
FWIW the only reference I could find for 9P requests that are not documented in
http://man.cat-v.org/plan_9/5/ is include/net/9p/9p.h in the linux kernel tree.
The official linux client has P9_TRENAME, P9_TRENAMEAT and P9_TMKDIR, but no
P9_TMKDIRAT...
pgp2XeaQC3oPj.pgp
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 0/5] 9P security fixes, Greg Kurz, 2016/08/26
- [Qemu-devel] [PATCH v2 1/5] 9p: forbid illegal path names, Greg Kurz, 2016/08/26
- [Qemu-devel] [PATCH v2 2/5] 9p: disallow the NUL character in all strings, Greg Kurz, 2016/08/26
- [Qemu-devel] [PATCH v2 3/5] 9p: forbid . and .. in file names, Greg Kurz, 2016/08/26
- [Qemu-devel] [PATCH v2 4/5] 9p: handle walk of ".." in the root directory, Greg Kurz, 2016/08/26
- [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Greg Kurz, 2016/08/26
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Eric Blake, 2016/08/26
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Michael S. Tsirkin, 2016/08/26
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Greg Kurz, 2016/08/28
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Greg Kurz, 2016/08/28
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Eric Blake, 2016/08/29
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Greg Kurz, 2016/08/30
- Re: [Qemu-devel] [PATCH v2 5/5] 9p: forbid empty extension string, Peter Maydell, 2016/08/28