| Message ID | 20210504124343.22611-1-khaledromdhani216@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-next] fs/cifs: Fix resource leak | expand |
On Tue, May 04, 2021 at 01:43:43PM +0100, Khaled ROMDHANI wrote: > The -EIO error return path is leaking memory allocated > to page. Fix this by invoking the free_dentry_path before > the return. > > Addresses-Coverity: ("Resource leak") > Signed-off-by: Khaled ROMDHANI <khaledromdhani216@gmail.com> Add a Fixes tag. Fixes: 583248493f78 ("cifs: add shutdown support") > --- > fs/cifs/link.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/link.c b/fs/cifs/link.c > index 1cbe7ec73728..1485c6095ba1 100644 > --- a/fs/cifs/link.c > +++ b/fs/cifs/link.c > @@ -686,8 +686,10 @@ cifs_symlink(struct user_namespace *mnt_userns, struct inode *inode, > void *page = alloc_dentry_path(); > struct inode *newinode = NULL; > > - if (unlikely(cifs_forced_shutdown(cifs_sb))) > + if (unlikely(cifs_forced_shutdown(cifs_sb))) { > + free_dentry_path(page); > return -EIO; > + } Better to move the allocation here. Avoid calling functions which can fail in the declaration block. Better to test for NULL directly instead of hiding the test inside the build_path_from_dentry() function. page = alloc_dentry_path(); if (!page) return -ENOMEM; The error handling in this function is slightly unweildy... regards, dan carpenter
On Tue, May 04, 2021 at 04:44:45PM +0300, Dan Carpenter wrote: > On Tue, May 04, 2021 at 01:43:43PM +0100, Khaled ROMDHANI wrote: > > The -EIO error return path is leaking memory allocated > > to page. Fix this by invoking the free_dentry_path before > > the return. > > > > Addresses-Coverity: ("Resource leak") > > Signed-off-by: Khaled ROMDHANI <khaledromdhani216@gmail.com> > > Add a Fixes tag. > > Fixes: 583248493f78 ("cifs: add shutdown support") > Yes, I will add a Fixes tag. > > --- > > fs/cifs/link.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/link.c b/fs/cifs/link.c > > index 1cbe7ec73728..1485c6095ba1 100644 > > --- a/fs/cifs/link.c > > +++ b/fs/cifs/link.c > > @@ -686,8 +686,10 @@ cifs_symlink(struct user_namespace *mnt_userns, struct inode *inode, > > void *page = alloc_dentry_path(); > > struct inode *newinode = NULL; > > > > - if (unlikely(cifs_forced_shutdown(cifs_sb))) > > + if (unlikely(cifs_forced_shutdown(cifs_sb))) { > > + free_dentry_path(page); > > return -EIO; > > + } > > Better to move the allocation here. Avoid calling functions which can > fail in the declaration block. Better to test for NULL directly instead > of hiding the test inside the build_path_from_dentry() function. > > page = alloc_dentry_path(); > if (!page) > return -ENOMEM; > > The error handling in this function is slightly unweildy... > > regards, > dan carpenter > Yes, it would be better to move the allocation... I will send a V2. Thanks.
diff --git a/fs/cifs/link.c b/fs/cifs/link.c index 1cbe7ec73728..1485c6095ba1 100644 --- a/fs/cifs/link.c +++ b/fs/cifs/link.c @@ -686,8 +686,10 @@ cifs_symlink(struct user_namespace *mnt_userns, struct inode *inode, void *page = alloc_dentry_path(); struct inode *newinode = NULL; - if (unlikely(cifs_forced_shutdown(cifs_sb))) + if (unlikely(cifs_forced_shutdown(cifs_sb))) { + free_dentry_path(page); return -EIO; + } xid = get_xid();
The -EIO error return path is leaking memory allocated to page. Fix this by invoking the free_dentry_path before the return. Addresses-Coverity: ("Resource leak") Signed-off-by: Khaled ROMDHANI <khaledromdhani216@gmail.com> --- fs/cifs/link.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)