Skip to content

Commit

Permalink
[PATCH] v9fs: fix races in fid allocation
Browse files Browse the repository at this point in the history
Fid management cleanup.  The patch attempts to fix the races in dentry's
fid management.

Dentries don't keep the opened fids anymore, they are moved to the file
structs.  Ideally there should be no more than one fid with fidcreate equal
to zero in the dentry's list of fids.

v9fs_fid_create initializes the important fields (fid, fidcreated) before
v9fs_fid is added to the list.  v9fs_fid_lookup returns only fids that are
not created by v9fs_create.  v9fs_fid_get_created returns the fid created
by the same process by v9fs_create (if any) and removes it from dentry's
list

Signed-off-by: Latchesar Ionkov <lucho@ionkov.net>
Cc: Eric Van Hensbergen <ericvh@gmail.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>
  • Loading branch information
Latchesar Ionkov authored and Linus Torvalds committed Sep 28, 2005
1 parent dc7b5fd commit 0b8dd17
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 196 deletions.
176 changes: 95 additions & 81 deletions fs/9p/fid.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,21 +71,28 @@ static int v9fs_fid_insert(struct v9fs_fid *fid, struct dentry *dentry)
*
*/

struct v9fs_fid *v9fs_fid_create(struct dentry *dentry)
struct v9fs_fid *v9fs_fid_create(struct dentry *dentry,
struct v9fs_session_info *v9ses, int fid, int create)
{
struct v9fs_fid *new;

dprintk(DEBUG_9P, "fid create dentry %p, fid %d, create %d\n",
dentry, fid, create);

new = kmalloc(sizeof(struct v9fs_fid), GFP_KERNEL);
if (new == NULL) {
dprintk(DEBUG_ERROR, "Out of Memory\n");
return ERR_PTR(-ENOMEM);
}

new->fid = -1;
new->fid = fid;
new->v9ses = v9ses;
new->fidopen = 0;
new->fidcreate = 0;
new->fidcreate = create;
new->fidclunked = 0;
new->iounit = 0;
new->rdir_pos = 0;
new->rdir_fcall = NULL;

if (v9fs_fid_insert(new, dentry) == 0)
return new;
Expand All @@ -108,6 +115,59 @@ void v9fs_fid_destroy(struct v9fs_fid *fid)
kfree(fid);
}

/**
* v9fs_fid_walk_up - walks from the process current directory
* up to the specified dentry.
*/
static struct v9fs_fid *v9fs_fid_walk_up(struct dentry *dentry)
{
int fidnum, cfidnum, err;
struct v9fs_fid *cfid;
struct dentry *cde;
struct v9fs_session_info *v9ses;

v9ses = v9fs_inode2v9ses(current->fs->pwd->d_inode);
cfid = v9fs_fid_lookup(current->fs->pwd);
if (cfid == NULL) {
dprintk(DEBUG_ERROR, "process cwd doesn't have a fid\n");
return ERR_PTR(-ENOENT);
}

cfidnum = cfid->fid;
cde = current->fs->pwd;
/* TODO: take advantage of multiwalk */

fidnum = v9fs_get_idpool(&v9ses->fidpool);
if (fidnum < 0) {
dprintk(DEBUG_ERROR, "could not get a new fid num\n");
err = -ENOENT;
goto clunk_fid;
}

while (cde != dentry) {
if (cde == cde->d_parent) {
dprintk(DEBUG_ERROR, "can't find dentry\n");
err = -ENOENT;
goto clunk_fid;
}

err = v9fs_t_walk(v9ses, cfidnum, fidnum, "..", NULL);
if (err < 0) {
dprintk(DEBUG_ERROR, "problem walking to parent\n");
goto clunk_fid;
}

cfidnum = fidnum;
cde = cde->d_parent;
}

return v9fs_fid_create(dentry, v9ses, fidnum, 0);

clunk_fid:
v9fs_t_clunk(v9ses, fidnum, NULL);
return ERR_PTR(err);
}

/**
* v9fs_fid_lookup - retrieve the right fid from a particular dentry
* @dentry: dentry to look for fid in
Expand All @@ -119,49 +179,25 @@ void v9fs_fid_destroy(struct v9fs_fid *fid)
*
*/

struct v9fs_fid *v9fs_fid_lookup(struct dentry *dentry, int type)
struct v9fs_fid *v9fs_fid_lookup(struct dentry *dentry)
{
struct list_head *fid_list = (struct list_head *)dentry->d_fsdata;
struct v9fs_fid *current_fid = NULL;
struct v9fs_fid *temp = NULL;
struct v9fs_fid *return_fid = NULL;
int found_parent = 0;
int found_user = 0;

dprintk(DEBUG_9P, " dentry: %s (%p) type %d\n", dentry->d_iname, dentry,
type);
dprintk(DEBUG_9P, " dentry: %s (%p)\n", dentry->d_iname, dentry);

if (fid_list && !list_empty(fid_list)) {
if (fid_list) {
list_for_each_entry_safe(current_fid, temp, fid_list, list) {
if (current_fid->uid == current->uid) {
if (return_fid == NULL) {
if ((type == FID_OP)
|| (!current_fid->fidopen)) {
return_fid = current_fid;
found_user = 1;
}
}
}
if (current_fid->pid == current->real_parent->pid) {
if ((return_fid == NULL) || (found_parent)
|| (found_user)) {
if ((type == FID_OP)
|| (!current_fid->fidopen)) {
return_fid = current_fid;
found_parent = 1;
found_user = 0;
}
}
}
if (current_fid->pid == current->pid) {
if ((type == FID_OP) ||
(!current_fid->fidopen)) {
return_fid = current_fid;
found_parent = 0;
found_user = 0;
}
if (!current_fid->fidcreate) {
return_fid = current_fid;
break;
}
}

if (!return_fid)
return_fid = current_fid;
}

/* we are at the root but didn't match */
Expand All @@ -187,55 +223,33 @@ struct v9fs_fid *v9fs_fid_lookup(struct dentry *dentry, int type)

/* XXX - there may be some duplication we can get rid of */
if (par == dentry) {
/* we need to fid_lookup the starting point */
int fidnum = -1;
int oldfid = -1;
int result = -1;
struct v9fs_session_info *v9ses =
v9fs_inode2v9ses(current->fs->pwd->d_inode);

current_fid =
v9fs_fid_lookup(current->fs->pwd, FID_WALK);
if (current_fid == NULL) {
dprintk(DEBUG_ERROR,
"process cwd doesn't have a fid\n");
return return_fid;
}
oldfid = current_fid->fid;
par = current->fs->pwd;
/* TODO: take advantage of multiwalk */
return_fid = v9fs_fid_walk_up(dentry);
if (IS_ERR(return_fid))
return_fid = NULL;
}
}

fidnum = v9fs_get_idpool(&v9ses->fidpool);
if (fidnum < 0) {
dprintk(DEBUG_ERROR,
"could not get a new fid num\n");
return return_fid;
}
return return_fid;
}

while (par != dentry) {
result =
v9fs_t_walk(v9ses, oldfid, fidnum, "..",
NULL);
if (result < 0) {
dprintk(DEBUG_ERROR,
"problem walking to parent\n");

break;
}
oldfid = fidnum;
if (par == par->d_parent) {
dprintk(DEBUG_ERROR,
"can't find dentry\n");
break;
}
par = par->d_parent;
}
if (par == dentry) {
return_fid = v9fs_fid_create(dentry);
return_fid->fid = fidnum;
struct v9fs_fid *v9fs_fid_get_created(struct dentry *dentry)
{
struct list_head *fid_list;
struct v9fs_fid *fid, *ftmp, *ret;

dprintk(DEBUG_9P, " dentry: %s (%p)\n", dentry->d_iname, dentry);
fid_list = (struct list_head *)dentry->d_fsdata;
ret = NULL;
if (fid_list) {
list_for_each_entry_safe(fid, ftmp, fid_list, list) {
if (fid->fidcreate && fid->pid == current->pid) {
list_del(&fid->list);
ret = fid;
break;
}
}
}

return return_fid;
dprintk(DEBUG_9P, "return %p\n", ret);
return ret;
}
7 changes: 5 additions & 2 deletions fs/9p/fid.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

#define FID_OP 0
#define FID_WALK 1
#define FID_CREATE 2

struct v9fs_fid {
struct list_head list; /* list of fids associated with a dentry */
Expand Down Expand Up @@ -52,6 +53,8 @@ struct v9fs_fid {
struct v9fs_session_info *v9ses; /* session info for this FID */
};

struct v9fs_fid *v9fs_fid_lookup(struct dentry *dentry, int type);
struct v9fs_fid *v9fs_fid_lookup(struct dentry *dentry);
struct v9fs_fid *v9fs_fid_get_created(struct dentry *);
void v9fs_fid_destroy(struct v9fs_fid *fid);
struct v9fs_fid *v9fs_fid_create(struct dentry *);
struct v9fs_fid *v9fs_fid_create(struct dentry *,
struct v9fs_session_info *v9ses, int fid, int create);
2 changes: 1 addition & 1 deletion fs/9p/vfs_dentry.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ static int v9fs_dentry_validate(struct dentry *dentry, struct nameidata *nd)
struct dentry *dc = current->fs->pwd;

dprintk(DEBUG_VFS, "dentry: %s (%p)\n", dentry->d_iname, dentry);
if (v9fs_fid_lookup(dentry, FID_OP)) {
if (v9fs_fid_lookup(dentry)) {
dprintk(DEBUG_VFS, "VALID\n");
return 1;
}
Expand Down
11 changes: 4 additions & 7 deletions fs/9p/vfs_dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,21 +197,18 @@ int v9fs_dir_release(struct inode *inode, struct file *filp)
filemap_fdatawait(inode->i_mapping);

if (fidnum >= 0) {
fid->fidopen--;
dprintk(DEBUG_VFS, "fidopen: %d v9f->fid: %d\n", fid->fidopen,
fid->fid);

if (fid->fidopen == 0) {
if (v9fs_t_clunk(v9ses, fidnum, NULL))
dprintk(DEBUG_ERROR, "clunk failed\n");
if (v9fs_t_clunk(v9ses, fidnum, NULL))
dprintk(DEBUG_ERROR, "clunk failed\n");

v9fs_put_idpool(fid->fid, &v9ses->fidpool);
}
v9fs_put_idpool(fid->fid, &v9ses->fidpool);

kfree(fid->rdir_fcall);
kfree(fid);

filp->private_data = NULL;
v9fs_fid_destroy(fid);
}

d_drop(filp->f_dentry);
Expand Down
Loading

0 comments on commit 0b8dd17

Please sign in to comment.