Skip to content

Commit

Permalink
nfsd: ensure that the ol stateid hash reference is only put once
Browse files Browse the repository at this point in the history 
When an open or lock stateid is hashed, we take an extra reference to
it. When we unhash it, we drop that reference. The code however does
not properly account for the case where we have two callers concurrently
trying to unhash the stateid. This can lead to list corruption and the
hash reference being put more than once.

Fix this by having unhash_ol_stateid use list_del_init on the st_perfile
list_head, and then testing to see if that list_head is empty before
releasing the hash reference. This means that some of the unhashing
wrappers now become bool return functions so we can test to see whether
the stateid was unhashed before we put the reference.

Reported-by: Andrew W Elble <[email protected]>
Tested-by: Andrew W Elble <[email protected]>
Reported-by: Anna Schumaker <[email protected]>
Tested-by: Anna Schumaker <[email protected]>
Signed-off-by: Jeff Layton <[email protected]>
Cc: [email protected]
Signed-off-by: J. Bruce Fields <[email protected]>
  • Loading branch information
@jtlayton
jtlayton authored and J. Bruce Fields committed Aug 31, 2015
1 parent 051ac38 commit e856873
Showing 1 changed file with 36 additions and 22 deletions.
58 changes: 36 additions & 22 deletions fs/nfsd/nfs4state.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1009,16 +1009,20 @@ static void nfs4_put_stateowner(struct nfs4_stateowner *sop)
nfs4_free_stateowner(sop);
}

static void unhash_ol_stateid(struct nfs4_ol_stateid *stp)
static bool unhash_ol_stateid(struct nfs4_ol_stateid *stp)
{
struct nfs4_file *fp = stp->st_stid.sc_file;

lockdep_assert_held(&stp->st_stateowner->so_client->cl_lock);

if (list_empty(&stp->st_perfile))
return false;

spin_lock(&fp->fi_lock);
list_del(&stp->st_perfile);
list_del_init(&stp->st_perfile);
spin_unlock(&fp->fi_lock);
list_del(&stp->st_perstateowner);
return true;
}

static void nfs4_free_ol_stateid(struct nfs4_stid *stid)
Expand Down Expand Up @@ -1068,25 +1072,27 @@ static void put_ol_stateid_locked(struct nfs4_ol_stateid *stp,
list_add(&stp->st_locks, reaplist);
}

static void unhash_lock_stateid(struct nfs4_ol_stateid *stp)
static bool unhash_lock_stateid(struct nfs4_ol_stateid *stp)
{
struct nfs4_openowner *oo = openowner(stp->st_openstp->st_stateowner);

lockdep_assert_held(&oo->oo_owner.so_client->cl_lock);

list_del_init(&stp->st_locks);
unhash_ol_stateid(stp);
nfs4_unhash_stid(&stp->st_stid);
return unhash_ol_stateid(stp);
}

static void release_lock_stateid(struct nfs4_ol_stateid *stp)
{
struct nfs4_openowner *oo = openowner(stp->st_openstp->st_stateowner);
bool unhashed;

spin_lock(&oo->oo_owner.so_client->cl_lock);
unhash_lock_stateid(stp);
unhashed = unhash_lock_stateid(stp);
spin_unlock(&oo->oo_owner.so_client->cl_lock);
nfs4_put_stid(&stp->st_stid);
if (unhashed)
nfs4_put_stid(&stp->st_stid);
}

static void unhash_lockowner_locked(struct nfs4_lockowner *lo)
Expand Down Expand Up @@ -1134,7 +1140,7 @@ static void release_lockowner(struct nfs4_lockowner *lo)
while (!list_empty(&lo->lo_owner.so_stateids)) {
stp = list_first_entry(&lo->lo_owner.so_stateids,
struct nfs4_ol_stateid, st_perstateowner);
unhash_lock_stateid(stp);
WARN_ON(!unhash_lock_stateid(stp));
put_ol_stateid_locked(stp, &reaplist);
}
spin_unlock(&clp->cl_lock);
Expand All @@ -1147,30 +1153,35 @@ static void release_open_stateid_locks(struct nfs4_ol_stateid *open_stp,
{
struct nfs4_ol_stateid *stp;

lockdep_assert_held(&open_stp->st_stid.sc_client->cl_lock);

while (!list_empty(&open_stp->st_locks)) {
stp = list_entry(open_stp->st_locks.next,
struct nfs4_ol_stateid, st_locks);
unhash_lock_stateid(stp);
WARN_ON(!unhash_lock_stateid(stp));
put_ol_stateid_locked(stp, reaplist);
}
}

static void unhash_open_stateid(struct nfs4_ol_stateid *stp,
static bool unhash_open_stateid(struct nfs4_ol_stateid *stp,
struct list_head *reaplist)
{
bool unhashed;

lockdep_assert_held(&stp->st_stid.sc_client->cl_lock);

unhash_ol_stateid(stp);
unhashed = unhash_ol_stateid(stp);
release_open_stateid_locks(stp, reaplist);
return unhashed;
}

static void release_open_stateid(struct nfs4_ol_stateid *stp)
{
LIST_HEAD(reaplist);

spin_lock(&stp->st_stid.sc_client->cl_lock);
unhash_open_stateid(stp, &reaplist);
put_ol_stateid_locked(stp, &reaplist);
if (unhash_open_stateid(stp, &reaplist))
put_ol_stateid_locked(stp, &reaplist);
spin_unlock(&stp->st_stid.sc_client->cl_lock);
free_ol_stateid_reaplist(&reaplist);
}
Expand Down Expand Up @@ -1215,8 +1226,8 @@ static void release_openowner(struct nfs4_openowner *oo)
while (!list_empty(&oo->oo_owner.so_stateids)) {
stp = list_first_entry(&oo->oo_owner.so_stateids,
struct nfs4_ol_stateid, st_perstateowner);
unhash_open_stateid(stp, &reaplist);
put_ol_stateid_locked(stp, &reaplist);
if (unhash_open_stateid(stp, &reaplist))
put_ol_stateid_locked(stp, &reaplist);
}
spin_unlock(&clp->cl_lock);
free_ol_stateid_reaplist(&reaplist);
Expand Down Expand Up @@ -4752,7 +4763,7 @@ nfsd4_free_stateid(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (check_for_locks(stp->st_stid.sc_file,
lockowner(stp->st_stateowner)))
break;
unhash_lock_stateid(stp);
WARN_ON(!unhash_lock_stateid(stp));
spin_unlock(&cl->cl_lock);
nfs4_put_stid(s);
ret = nfs_ok;
Expand Down Expand Up @@ -4968,20 +4979,23 @@ nfsd4_open_downgrade(struct svc_rqst *rqstp,
static void nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
{
struct nfs4_client *clp = s->st_stid.sc_client;
bool unhashed;
LIST_HEAD(reaplist);

s->st_stid.sc_type = NFS4_CLOSED_STID;
spin_lock(&clp->cl_lock);
unhash_open_stateid(s, &reaplist);
unhashed = unhash_open_stateid(s, &reaplist);

if (clp->cl_minorversion) {
put_ol_stateid_locked(s, &reaplist);
if (unhashed)
put_ol_stateid_locked(s, &reaplist);
spin_unlock(&clp->cl_lock);
free_ol_stateid_reaplist(&reaplist);
} else {
spin_unlock(&clp->cl_lock);
free_ol_stateid_reaplist(&reaplist);
move_to_close_lru(s, clp->net);
if (unhashed)
move_to_close_lru(s, clp->net);
}
}

Expand Down Expand Up @@ -6014,7 +6028,7 @@ nfsd_inject_add_lock_to_list(struct nfs4_ol_stateid *lst,

static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
struct list_head *collect,
void (*func)(struct nfs4_ol_stateid *))
bool (*func)(struct nfs4_ol_stateid *))
{
struct nfs4_openowner *oop;
struct nfs4_ol_stateid *stp, *st_next;
Expand All @@ -6028,9 +6042,9 @@ static u64 nfsd_foreach_client_lock(struct nfs4_client *clp, u64 max,
list_for_each_entry_safe(lst, lst_next,
&stp->st_locks, st_locks) {
if (func) {
func(lst);
nfsd_inject_add_lock_to_list(lst,
collect);
if (func(lst))
nfsd_inject_add_lock_to_list(lst,
collect);
}
++count;
/*
Expand Down

0 comments on commit e856873

Please sign in to comment.