LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] cifs: dir: fix memory leak in cifs_mknod
@ 2018-04-20 13:19 Gustavo A. R. Silva
  2018-04-20 17:37 ` Steve French
  0 siblings, 1 reply; 4+ messages in thread
From: Gustavo A. R. Silva @ 2018-04-20 13:19 UTC (permalink / raw)
  To: Ronnie Sahlberg, Steve French
  Cc: linux-cifs, samba-technical, linux-kernel, Gustavo A. R. Silva

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: dir: fix memory leak in cifs_mknod
  2018-04-20 13:19 [PATCH] cifs: dir: fix memory leak in cifs_mknod Gustavo A. R. Silva
@ 2018-04-20 17:37 ` Steve French
  2018-04-20 17:48   ` Gustavo A. R. Silva
  2018-04-20 18:02   ` Pavel Shilovsky
  0 siblings, 2 replies; 4+ messages in thread
From: Steve French @ 2018-04-20 17:37 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Colin King
  Cc: Ronnie Sahlberg, Steve French, CIFS, samba-technical, LKML,
	kernel-janitors

[-- Attachment #1: Type: text/plain, Size: 1638 bytes --]

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



-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-do-not-allow-creating-sockets-except-with-SMB1-.patch --]
[-- Type: text/x-patch, Size: 2395 bytes --]

From 167bc5de08dc97695f9d5c7069c3e69f409ff80b Mon Sep 17 00:00:00 2001
From: Steve French <smfrench@gmail.com>
Date: Fri, 20 Apr 2018 12:19:07 -0500
Subject: [PATCH 1/2] cifs: do not allow creating sockets except with SMB1
 posix exensions

RHBZ: 1453123

Since at least the 3.10 kernel and likely a lot earlier we have
not been able to create unix domain sockets in a cifs share
when mounted using the SFU mount option (except when mounted
with the cifs unix extensions to Samba e.g.)
Trying to create a socket, for example using the af_unix command from
xfstests will cause :
BUG: unable to handle kernel NULL pointer dereference at 00000000
00000040

Since no one uses or depends on being able to create unix domains sockets
on a cifs share the easiest fix to stop this vulnerability is to simply
not allow creation of any other special files than char or block devices
when sfu is used.

Added update to Ronnie's patch to handle a tcon link leak, and
to address a buf leak noticed by Gustavo and Colin.

CC: Gustavo A. R. Silva <gustavo@embeddedor.com>
CC:  Colin Ian King <colin.king@canonical.com>
Reported-by: Eryu Guan <eguan@redhat.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Cc: stable@vger.kernel.org
---
 fs/cifs/dir.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 81ba6e0d88d8..925844343038 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -684,6 +684,9 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
 		goto mknod_out;
 	}
 
+	if (!S_ISCHR(mode) && !S_ISBLK(mode))
+		goto mknod_out;
+
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
 		goto mknod_out;
 
@@ -692,10 +695,8 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
 
 	buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
 	if (buf == NULL) {
-		kfree(full_path);
 		rc = -ENOMEM;
-		free_xid(xid);
-		return rc;
+		goto mknod_out;
 	}
 
 	if (backup_cred(cifs_sb))
@@ -742,7 +743,7 @@ int cifs_mknod(struct inode *inode, struct dentry *direntry, umode_t mode,
 		pdev->minor = cpu_to_le64(MINOR(device_number));
 		rc = tcon->ses->server->ops->sync_write(xid, &fid, &io_parms,
 							&bytes_written, iov, 1);
-	} /* else if (S_ISFIFO) */
+	}
 	tcon->ses->server->ops->close(xid, tcon, &fid);
 	d_drop(direntry);
 
-- 
2.14.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: dir: fix memory leak in cifs_mknod
  2018-04-20 17:37 ` Steve French
@ 2018-04-20 17:48   ` Gustavo A. R. Silva
  2018-04-20 18:02   ` Pavel Shilovsky
  1 sibling, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2018-04-20 17:48 UTC (permalink / raw)
  To: Steve French, Colin King
  Cc: Ronnie Sahlberg, Steve French, CIFS, samba-technical, LKML,
	kernel-janitors

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
> 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] cifs: dir: fix memory leak in cifs_mknod
  2018-04-20 17:37 ` Steve French
  2018-04-20 17:48   ` Gustavo A. R. Silva
@ 2018-04-20 18:02   ` Pavel Shilovsky
  1 sibling, 0 replies; 4+ messages in thread
From: Pavel Shilovsky @ 2018-04-20 18:02 UTC (permalink / raw)
  To: Steve French
  Cc: Gustavo A. R. Silva, Colin King, Ronnie Sahlberg, Steve French,
	CIFS, samba-technical, LKML, kernel-janitors

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-20 18:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-20 13:19 [PATCH] cifs: dir: fix memory leak in cifs_mknod Gustavo A. R. Silva
2018-04-20 17:37 ` Steve French
2018-04-20 17:48   ` Gustavo A. R. Silva
2018-04-20 18:02   ` Pavel Shilovsky

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).