cifs: dir: fix memory leak in cifs_mknod

Message ID 20180420131919.GA1766@embeddedor.com
State New
Headers show
Series
  • cifs: dir: fix memory leak in cifs_mknod
Related show

Commit Message

Gustavo A. R. Silva April 20, 2018, 1:19 p.m.
Free allocated memory for full_path and xid before return.

Addresses-Coverity-ID: 1468029 ("Resource leak")
Fixes: 49162bfde140 ("cifs: do not allow creating sockets except with
SMB1 posix exensions")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 fs/cifs/dir.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Steve French April 20, 2018, 5:37 p.m. | #1
I noticed a similar problem with the tcon link leak on that (which
Colin and Gustavo pointed out - thank you!) but also in another return
statement, so updated the original patch of Ronnie's merging the fixes

https://git.samba.org/sfrench/cifs-2.6.git/?p=sfrench/cifs-2.6.git;a=commit;h=167bc5de08dc97695f9d5c7069c3e69f409ff80b


Let me know if you see any problems with it.

On Fri, Apr 20, 2018 at 8:19 AM, Gustavo A. R. Silva
<gustavo@embeddedor.com> wrote:
> Free allocated memory for full_path and xid before return.
>
> Addresses-Coverity-ID: 1468029 ("Resource leak")
> Fixes: 49162bfde140 ("cifs: do not allow creating sockets except with
> SMB1 posix exensions")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  fs/cifs/dir.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index f0a759d..71e32d9 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -684,8 +684,11 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
>                 goto mknod_out;
>         }
>
> -       if (!S_ISCHR(mode) && !S_ISBLK(mode))
> +       if (!S_ISCHR(mode) && !S_ISBLK(mode)) {
> +               kfree(full_path);
> +               free_xid(xid);
>                 return -EPERM;
> +       }
>
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
>                 goto mknod_out;
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gustavo A. R. Silva April 20, 2018, 5:48 p.m. | #2
Hi Steve,

On 04/20/2018 12:37 PM, Steve French wrote:
> I noticed a similar problem with the tcon link leak on that (which
> Colin and Gustavo pointed out - thank you!) but also in another return
> statement, so updated the original patch of Ronnie's merging the fixes
> 
> https://git.samba.org/sfrench/cifs-2.6.git/?p=sfrench/cifs-2.6.git;a=commit;h=167bc5de08dc97695f9d5c7069c3e69f409ff80b
> 
> 
> Let me know if you see any problems with it.
> 

Looks correct to me.

Thanks
--
Gustavo

> On Fri, Apr 20, 2018 at 8:19 AM, Gustavo A. R. Silva
> <gustavo@embeddedor.com> wrote:
>> Free allocated memory for full_path and xid before return.
>>
>> Addresses-Coverity-ID: 1468029 ("Resource leak")
>> Fixes: 49162bfde140 ("cifs: do not allow creating sockets except with
>> SMB1 posix exensions")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>   fs/cifs/dir.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
>> index f0a759d..71e32d9 100644
>> --- a/fs/cifs/dir.c
>> +++ b/fs/cifs/dir.c
>> @@ -684,8 +684,11 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
>>                  goto mknod_out;
>>          }
>>
>> -       if (!S_ISCHR(mode) && !S_ISBLK(mode))
>> +       if (!S_ISCHR(mode) && !S_ISBLK(mode)) {
>> +               kfree(full_path);
>> +               free_xid(xid);
>>                  return -EPERM;
>> +       }
>>
>>          if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
>>                  goto mknod_out;
>> --
>> 2.7.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Shilovsky April 20, 2018, 6:02 p.m. | #3
2018-04-20 10:37 GMT-07:00 Steve French <smfrench@gmail.com>:
> I noticed a similar problem with the tcon link leak on that (which
> Colin and Gustavo pointed out - thank you!) but also in another return
> statement, so updated the original patch of Ronnie's merging the fixes
>
> https://git.samba.org/sfrench/cifs-2.6.git/?p=sfrench/cifs-2.6.git;a=commit;h=167bc5de08dc97695f9d5c7069c3e69f409ff80b
>
>
> Let me know if you see any problems with it.
>

Looks good.

Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>

--
Best regards,
Pavel Shilovsky
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index f0a759d..71e32d9 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -684,8 +684,11 @@  int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
 		goto mknod_out;
 	}
 
-	if (!S_ISCHR(mode) && !S_ISBLK(mode))
+	if (!S_ISCHR(mode) && !S_ISBLK(mode)) {
+		kfree(full_path);
+		free_xid(xid);
 		return -EPERM;
+	}
 
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
 		goto mknod_out;