Zhihao Cheng <[email protected]> wrote:
> sysnames should be freed after refcnt being decreased to zero in
> afs_put_sysnames().
Good catch.
> Besides, it would be better set net->sysnames to 'NULL' after net->sysnames
> being released if afs_put_sysnames() aims on an afs_sysnames object.
Why? We don't normally clear pointers when cleaning up a struct - and of the
two places this is relevant, in one we fail to set up a namespace and in the
other we're tearing down a namespace. In neither case should the pointer be
accessed again.
> @@ -894,7 +894,7 @@ static struct dentry *afs_lookup_atsys(struct inode *dir,
> struct dentry *dentry,
> */
> ret = NULL;
> out_s:
> - afs_put_sysnames(subs);
> + afs_put_sysnames_and_null(net);
This is definitely wrong. We obtained a ref 23 lines above and dropped the
lock:
read_lock(&net->sysnames_lock);
subs = net->sysnames;
refcount_inc(&subs->usage);
read_unlock(&net->sysnames_lock);
We are dropping *that* ref, not the one in the struct.
Just adding the missing kfree() call into afs_put_sysnames() should suffice,
thanks.
David
