On Sat, 23 Jul 2011 05:31:20 +0100, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > On Fri, Jul 22, 2011 at 08:42:35PM -0700, Linus Torvalds wrote: > > > ... why is that a problem? ?Locking there is mere ->i_lock and getting > > > a refcount is atomic_inc(). ?Grabbing a reference might be Not Nice from > > > the cacheline bouncing POV, but... > > > > I agree in theory, but it's not something we've done before. Some of > > the posix-acl code is pretty disgusting, I didn't even want to go > > there. > > > > And the case that tends to really *matter* is the "no acls" case > > anyway, and that's the case that is guaranteed to have no nasty races > > or odd issues with having to allocate/de-allocate any acl structures. > > But yes, this could be looked at. > > Heh... In addition to ocfs2 leak: 9p leaks nicely if v9fs_acl_mode() is > called with !S_ISDIR(mode). In that case acl reference is simply lost. > So yes, it's worth looking at. something like the below ? I will also audit rest of the code. diff --git a/fs/9p/acl.c b/fs/9p/acl.c index e98f56d..abad8b3 100644 --- a/fs/9p/acl.c +++ b/fs/9p/acl.c @@ -212,7 +212,7 @@ int v9fs_acl_mode(struct inode *dir, mode_t *modep, struct posix_acl *clone; if (S_ISDIR(mode)) - *dpacl = acl; + *dpacl = posix_acl_clone(acl, GFP_NOFS); clone = posix_acl_clone(acl, GFP_NOFS); retval = -ENOMEM; if (!clone) @@ -227,7 +227,7 @@ int v9fs_acl_mode(struct inode *dir, mode_t *modep, *pacl = clone; } *modep = mode; - return 0; + retval = 0; cleanup: posix_acl_release(acl); return retval; -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
