Messages in this thread | | | From | Trond Myklebust <> | Subject | Re: [PATCH] nfs: Fix potential posix_acl refcnt leak in nfs3_set_acl | Date | Mon, 20 Apr 2020 12:14:07 +0000 |
| |
On Mon, 2020-04-20 at 13:43 +0800, Xiyu Yang wrote: > nfs3_set_acl() invokes get_acl(), which returns a local reference of > the > posix_acl object to "alloc" with increased refcount. > > When nfs3_set_acl() returns or a new object is assigned to > "alloc", the > original local reference of "alloc" becomes invalid, so the refcount > should be decreased to keep refcount balanced. > > The reference counting issue happens in one path of nfs3_set_acl(). > When > "acl" equals to NULL but "alloc" is not NULL, the function forgets to > decrease the refcnt increased by get_acl() and causes a refcnt leak. > > Fix this issue by calling posix_acl_release() on this path when > "alloc" > is not NULL. > > Fixes: b7fa0554cf1b ("[PATCH] NFS: Add support for NFSv3 ACLs") > Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn> > Signed-off-by: Xin Tan <tanxin.ctf@gmail.com> > --- > fs/nfs/nfs3acl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c > index c5c3fc6e6c60..b5c41bcca8cf 100644 > --- a/fs/nfs/nfs3acl.c > +++ b/fs/nfs/nfs3acl.c > @@ -274,6 +274,8 @@ int nfs3_set_acl(struct inode *inode, struct > posix_acl *acl, int type) > } > > if (acl == NULL) { > + if (alloc) > + posix_acl_release(alloc);
This will result in 'dfacl' being freed while it is still in use, so can't be correct either.
> alloc = acl = posix_acl_from_mode(inode->i_mode, > GFP_KERNEL); > if (IS_ERR(alloc)) > goto fail;
I don't really see any alternative to adding a 'dfalloc' to track the allocated dfacl separately.
-- Trond Myklebust Linux NFS client maintainer, Hammerspace trond.myklebust@hammerspace.com
| |