Anders Hammarquist wrote:
> I just booted 2.1.105 (Linus version) and found a rather interesting
> misfeature with knfs. My client (sun sparcclassic running linux 2.0.33) did
> mkdir . in a nfsmounted directory (tar extracting an archive). This caused
> knfsd to make a new directory of the same name as the old, in the old
> directory, but link it in to the dcache where the old directory had been. A
> bit frightening until I ran debugfs on the filesystem to find out what really
> happened...
>
> Anyway, attached is a short patch that makes attempts to create existing
> directories over nfs fail with -EEXIST. Tested here and is working fine so far.
Hi Anders,
I posted a patch for knfsd a while back that corrects the knfsd directory locking
problems. It should take care of the mkdir problem you noted, as well as other
similar cases.
I've attached a copy against 2.1.105.
Regards,
Bill
--------------40677FC27AC4D4892137C63B
Content-Type: text/plain; charset=us-ascii; name="nfsd_105-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="nfsd_105-patch"
--- linux-2.1.105/include/linux/nfsd/nfsd.h.old Tue Mar 17 21:45:02 1998
+++ linux-2.1.105/include/linux/nfsd/nfsd.h Sun Jun 7 15:28:27 1998
@@ -60,6 +60,7 @@
};
typedef int (*encode_dent_fn)(struct readdir_cd *, const char *,
int, off_t, ino_t);
+typedef int (*nfsd_dirop_t)(struct inode *, struct dentry *, int, int);
/*
* Procedure table for NFSv2
@@ -72,6 +73,8 @@
*/
int nfsd_svc(unsigned short port, int nrservs);
+/* nfsd/vfs.c */
+int fh_lock_parent(struct svc_fh *, struct dentry *);
void nfsd_racache_init(void);
int nfsd_lookup(struct svc_rqst *, struct svc_fh *,
const char *, int, struct svc_fh *);
--- linux-2.1.105/fs/nfsd/vfs.c.old Thu Jun 4 18:08:29 1998
+++ linux-2.1.105/fs/nfsd/vfs.c Sun Jun 7 15:00:00 1998
@@ -31,14 +31,13 @@
#include <linux/sunrpc/svc.h>
#include <linux/nfsd/nfsd.h>
+#include <linux/nfsd/nfsfh.h>
#include <linux/quotaops.h>
#if LINUX_VERSION_CODE >= 0x020100
#include <asm/uaccess.h>
#endif
-extern void fh_update(struct svc_fh*);
-
#define NFSDDBG_FACILITY NFSDDBG_FILEOP
/* Open mode for nfsd_open */
@@ -77,6 +76,41 @@
static struct raparms * raparm_cache = 0;
/*
+ * Lock a parent directory following the VFS locking protocol.
+ */
+int
+fh_lock_parent(struct svc_fh *parent_fh, struct dentry *dchild)
+{
+ int nfserr = 0;
+
+ fh_lock(parent_fh);
+ /*
+ * Make sure the parent->child relationship still holds,
+ * and that the child is still hashed.
+ */
+ if (dchild->d_parent != parent_fh->fh_dentry)
+ goto out_not_parent;
+ if (list_empty(&dchild->d_hash))
+ goto out_not_hashed;
+out:
+ return nfserr;
+
+out_not_parent:
+ printk(KERN_WARNING
+ "fh_lock_parent: %s/%s parent changed\n",
+ dchild->d_parent->d_name.name, dchild->d_name.name);
+ goto out_unlock;
+out_not_hashed:
+ printk(KERN_WARNING
+ "fh_lock_parent: %s/%s unhashed\n",
+ dchild->d_parent->d_name.name, dchild->d_name.name);
+out_unlock:
+ nfserr = nfserr_noent;
+ fh_unlock(parent_fh);
+ goto out;
+}
+
+/*
* Deny access to certain file systems
*/
static inline int
@@ -546,8 +580,11 @@
}
/*
- * Create a file (regular, directory, device, fifo).
- * UNIX sockets not yet implemented.
+ * Create a file (regular, directory, device, fifo); UNIX sockets
+ * not yet implemented.
+ * If the response fh has been verified, the parent directory should
+ * already be locked. Note that the parent directory is left locked.
+ *
* N.B. Every call to nfsd_create needs an fh_put for _both_ fhp and resfhp
*/
int
@@ -557,13 +594,12 @@
{
struct dentry *dentry, *dchild;
struct inode *dirp;
+ nfsd_dirop_t opfunc = NULL;
int err;
err = nfserr_perm;
if (!flen)
goto out;
- if (!(iap->ia_valid & ATTR_MODE))
- iap->ia_mode = 0;
err = fh_verify(rqstp, fhp, S_IFDIR, MAY_CREATE);
if (err)
goto out;
@@ -571,64 +607,74 @@
dentry = fhp->fh_dentry;
dirp = dentry->d_inode;
- /* Get all the sanity checks out of the way before we lock the parent. */
err = nfserr_notdir;
if(!dirp->i_op || !dirp->i_op->lookup)
goto out;
- err = nfserr_perm;
- if (type == S_IFREG) {
- if(!dirp->i_op->create)
- goto out;
- } else if(type == S_IFDIR) {
- if(!dirp->i_op->mkdir)
- goto out;
- } else if((type == S_IFCHR) || (type == S_IFBLK) || (type == S_IFIFO)) {
- if(!dirp->i_op->mknod)
- goto out;
- } else {
- goto out;
- }
-
/*
- * The response filehandle may have been setup already ...
+ * Check whether the response filehandle has been verified yet.
+ * If it has, the parent directory should already be locked.
*/
if (!resfhp->fh_dverified) {
dchild = lookup_dentry(fname, dget(dentry), 0);
err = PTR_ERR(dchild);
- if(IS_ERR(dchild))
+ if (IS_ERR(dchild))
goto out_nfserr;
fh_compose(resfhp, fhp->fh_export, dchild);
- } else
+ /* Lock the parent and check for errors ... */
+ err = fh_lock_parent(fhp, dchild);
+ if (err)
+ goto out;
+ } else {
dchild = resfhp->fh_dentry;
+ if (!fhp->fh_locked)
+ printk(KERN_ERR
+ "nfsd_create: parent %s/%s not locked!\n",
+ dentry->d_parent->d_name.name,
+ dentry->d_name.name);
+ }
/*
* Make sure the child dentry is still negative ...
*/
+ err = nfserr_exist;
if (dchild->d_inode) {
- printk("nfsd_create: dentry %s/%s not negative!\n",
- dentry->d_parent->d_name.name, dentry->d_name.name);
+ printk(KERN_WARNING
+ "nfsd_create: dentry %s/%s not negative!\n",
+ dentry->d_name.name, dchild->d_name.name);
+ goto out;
}
- /* Looks good, lock the directory. */
- fh_lock(fhp);
- DQUOT_INIT(dirp);
+ /*
+ * Get the dir op function pointer.
+ */
+ err = nfserr_perm;
switch (type) {
case S_IFREG:
- err = dirp->i_op->create(dirp, dchild, iap->ia_mode);
+ opfunc = (nfsd_dirop_t) dirp->i_op->create;
break;
case S_IFDIR:
- err = dirp->i_op->mkdir(dirp, dchild, iap->ia_mode);
+ opfunc = (nfsd_dirop_t) dirp->i_op->mkdir;
break;
case S_IFCHR:
case S_IFBLK:
case S_IFIFO:
- err = dirp->i_op->mknod(dirp, dchild, iap->ia_mode, rdev);
+ opfunc = dirp->i_op->mknod;
break;
}
- DQUOT_DROP(dirp);
- fh_unlock(fhp);
+ if (!opfunc)
+ goto out;
+ if (!(iap->ia_valid & ATTR_MODE))
+ iap->ia_mode = 0;
+
+ /*
+ * Call the dir op function to create the object.
+ */
+ DQUOT_INIT(dirp);
+ err = opfunc(dirp, dchild, iap->ia_mode, rdev);
+ DQUOT_DROP(dirp);
if (err < 0)
goto out_nfserr;
+
if (EX_ISSYNC(fhp->fh_export))
write_inode_now(dirp);
@@ -778,21 +824,29 @@
if (IS_ERR(dnew))
goto out_nfserr;
- err = -EEXIST;
+ /*
+ * Lock the parent before checking for existence
+ */
+ err = fh_lock_parent(fhp, dnew);
+ if (err)
+ goto out_compose;
+
+ err = nfserr_exist;
if (!dnew->d_inode) {
- fh_lock(fhp);
DQUOT_INIT(dirp);
err = dirp->i_op->symlink(dirp, dnew, path);
DQUOT_DROP(dirp);
- fh_unlock(fhp);
if (!err) {
if (EX_ISSYNC(fhp->fh_export))
write_inode_now(dirp);
- }
+ } else
+ err = nfserrno(-err);
}
+ fh_unlock(fhp);
+
+ /* Compose the fh so the dentry will be freed ... */
+out_compose:
fh_compose(resfhp, fhp->fh_export, dnew);
- if (err)
- goto out_nfserr;
out:
return err;
@@ -820,6 +874,10 @@
if (err)
goto out;
+ err = nfserr_perm;
+ if (!len)
+ goto out;
+
ddir = ffhp->fh_dentry;
dirp = ddir->d_inode;
@@ -827,44 +885,48 @@
err = PTR_ERR(dnew);
if (IS_ERR(dnew))
goto out_nfserr;
+ /*
+ * Lock the parent before checking for existence
+ */
+ err = fh_lock_parent(ffhp, dnew);
+ if (err)
+ goto out_dput;
- err = -EEXIST;
+ err = nfserr_exist;
if (dnew->d_inode)
- goto dput_and_out;
-
- err = -EPERM;
- if (!len)
- goto dput_and_out;
+ goto out_unlock;
dold = tfhp->fh_dentry;
dest = dold->d_inode;
- err = -EACCES;
+ err = nfserr_acces;
if (nfsd_iscovered(ddir, ffhp->fh_export))
- goto dput_and_out;
+ goto out_unlock;
+ /* FIXME: nxdev for NFSv3 */
if (dirp->i_dev != dest->i_dev)
- goto dput_and_out; /* FIXME: nxdev for NFSv3 */
+ goto out_unlock;
- err = -EPERM;
+ err = nfserr_perm;
if (IS_IMMUTABLE(dest) /* || IS_APPEND(dest) */ )
- goto dput_and_out;
+ goto out_unlock;
if (!dirp->i_op || !dirp->i_op->link)
- goto dput_and_out;
+ goto out_unlock;
- fh_lock(ffhp);
DQUOT_INIT(dirp);
err = dirp->i_op->link(dold, dirp, dnew);
DQUOT_DROP(dirp);
- fh_unlock(ffhp);
+ if (!err) {
+ if (EX_ISSYNC(ffhp->fh_export)) {
+ write_inode_now(dirp);
+ write_inode_now(dest);
+ }
+ } else
+ err = nfserrno(-err);
- if (!err && EX_ISSYNC(ffhp->fh_export)) {
- write_inode_now(dirp);
- write_inode_now(dest);
- }
-dput_and_out:
+out_unlock:
+ fh_unlock(ffhp);
+out_dput:
dput(dnew);
- if (err)
- goto out_nfserr;
out:
return err;
@@ -1005,7 +1067,10 @@
if (IS_ERR(rdentry))
goto out_nfserr;
- fh_lock(fhp);
+ err = fh_lock_parent(fhp, rdentry);
+ if (err)
+ goto out;
+
DQUOT_INIT(dirp);
if (type == S_IFDIR) {
err = -ENOTDIR;
--- linux-2.1.105/fs/nfsd/nfsproc.c.old Sun Jan 25 10:07:33 1998
+++ linux-2.1.105/fs/nfsd/nfsproc.c Sun Jun 7 15:00:00 1998
@@ -180,10 +180,8 @@
/*
* CREATE processing is complicated. The keyword here is `overloaded.'
- * There's a small race condition here between the check for existence
- * and the actual create() call, but one could even consider this a
- * feature because this only happens if someone else creates the file
- * at the same time.
+ * The parent directory is kept locked between the check for existence
+ * and the actual create() call in compliance with VFS protocols.
* N.B. After this call _both_ argp->fh and resp->fh need an fh_put
*/
static int
@@ -193,15 +191,14 @@
svc_fh *dirfhp = &argp->fh;
svc_fh *newfhp = &resp->fh;
struct iattr *attr = &argp->attrs;
- struct inode *inode = NULL;
- int nfserr, type, mode;
- int rdonly = 0, exists;
+ struct inode *inode;
+ int nfserr, type, mode, rdonly = 0;
dev_t rdev = NODEV;
dprintk("nfsd: CREATE %d/%ld %s\n",
SVCFH_DEV(dirfhp), SVCFH_INO(dirfhp), argp->name);
- /* Get the directory inode */
+ /* First verify the parent filehandle */
nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, MAY_EXEC);
if (nfserr)
goto done; /* must fh_put dirfhp even on error */
@@ -214,18 +211,32 @@
} else if (nfserr)
goto done;
- /* First, check if the file already exists. */
- exists = !nfsd_lookup(rqstp, dirfhp, argp->name, argp->len, newfhp);
-
- if (newfhp->fh_dverified)
- inode = newfhp->fh_dentry->d_inode;
+ /*
+ * Do a lookup to verify the new filehandle.
+ */
+ nfserr = nfsd_lookup(rqstp, dirfhp, argp->name, argp->len, newfhp);
+ if (nfserr) {
+ if (nfserr != nfserr_noent)
+ goto done;
+ /*
+ * If the new filehandle wasn't verified, we can't tell
+ * whether the file exists or not. Time to bail ...
+ */
+ nfserr = nfserr_acces;
+ if (!newfhp->fh_dverified) {
+ printk(KERN_WARNING
+ "nfsd_proc_create: filehandle not verified\n");
+ goto done;
+ }
+ }
- /* Get rid of this soon... */
- if (exists && !inode) {
- printk("nfsd_proc_create: Wheee... exists but d_inode==NULL\n");
- nfserr = nfserr_rofs;
+ /*
+ * Lock the parent directory and check for existence.
+ */
+ nfserr = fh_lock_parent(dirfhp, newfhp->fh_dentry);
+ if (nfserr)
goto done;
- }
+ inode = newfhp->fh_dentry->d_inode;
/* Unfudge the mode bits */
if (attr->ia_valid & ATTR_MODE) {
@@ -233,7 +244,7 @@
mode = attr->ia_mode & ~S_IFMT;
if (!type) /* HP weirdness */
type = S_IFREG;
- } else if (exists) {
+ } else if (inode) {
type = inode->i_mode & S_IFMT;
mode = inode->i_mode & ~S_IFMT;
} else {
@@ -243,8 +254,8 @@
/* This is for "echo > /dev/null" a la SunOS. Argh. */
nfserr = nfserr_rofs;
- if (rdonly && (!exists || type == S_IFREG))
- goto done;
+ if (rdonly && (!inode || type == S_IFREG))
+ goto out_unlock;
attr->ia_valid |= ATTR_MODE;
attr->ia_mode = type | mode;
@@ -252,7 +263,6 @@
/* Special treatment for non-regular files according to the
* gospel of sun micro
*/
- nfserr = 0;
if (type != S_IFREG) {
int is_borc = 0;
u32 size = attr->ia_size;
@@ -267,21 +277,21 @@
} else if (size != rdev) {
/* dev got truncated because of 16bit Linux dev_t */
nfserr = nfserr_io; /* or nfserr_inval? */
- goto done;
+ goto out_unlock;
} else {
/* Okay, char or block special */
is_borc = 1;
}
/* Make sure the type and device matches */
- if (exists && (type != (inode->i_mode & S_IFMT)
- || (is_borc && inode->i_rdev != rdev))) {
- nfserr = nfserr_exist;
- goto done;
- }
+ nfserr = nfserr_exist;
+ if (inode && (type != (inode->i_mode & S_IFMT) ||
+ (is_borc && inode->i_rdev != rdev)))
+ goto out_unlock;
}
- if (!exists) {
+ nfserr = 0;
+ if (!inode) {
/* File doesn't exist. Create it and set attrs */
nfserr = nfsd_create(rqstp, dirfhp, argp->name, argp->len,
attr, type, rdev, newfhp);
@@ -297,6 +307,10 @@
nfserr = nfsd_setattr(rqstp, newfhp, attr);
}
+out_unlock:
+ /* We don't really need to unlock, as fh_put does it. */
+ fh_unlock(dirfhp);
+
done:
fh_put(dirfhp);
RETURN(nfserr);
@@ -359,9 +373,9 @@
int nfserr;
dprintk("nfsd: SYMLINK %p %s -> %s\n",
- SVCFH_DENTRY(&argp->ffh),
- argp->fname, argp->tname);
+ SVCFH_DENTRY(&argp->ffh), argp->fname, argp->tname);
+ memset(&newfh, 0, sizeof(struct svc_fh));
/*
* Create the link, look up new file and set attrs.
*/
@@ -386,12 +400,13 @@
{
int nfserr;
- dprintk("nfsd: MKDIR %p %s\n",
- SVCFH_DENTRY(&argp->fh),
- argp->name);
+ dprintk("nfsd: MKDIR %p %s\n", SVCFH_DENTRY(&argp->fh), argp->name);
+
+ if (resp->fh.fh_dverified) {
+ printk(KERN_WARNING
+ "nfsd_proc_mkdir: response already verified??\n");
+ }
- /* N.B. what about the dentry count?? */
- resp->fh.fh_dverified = 0; /* paranoia */
nfserr = nfsd_create(rqstp, &argp->fh, argp->name, argp->len,
&argp->attrs, S_IFDIR, 0, &resp->fh);
fh_put(&argp->fh);
--------------40677FC27AC4D4892137C63B--
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu