[PATCH] cifs: fix oops on mount when CONFIG_CIFS_DFS_UPCALL is enabled

[PATCH] cifs: fix oops on mount when CONFIG_CIFS_DFS_UPCALL is enabled

[Marcin Slusarz]

simple "mount -t cifs //xxx /mnt" oopsed on strlen of options
http://kerneloops.org/guilty.php?guilty=cifs_get_sb&version=2.6.25-release&start=1671168&end=1703935&class=oops

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: Steve French <sfrench@samba.org>
Cc: linux-cifs-client@lists.samba.org
Cc: stable@kernel.org
---
 fs/cifs/cifsfs.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index 5df93fd..86b4d5f 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -97,9 +97,6 @@ cifs_read_super(struct super_block *sb, void *data,
 {
 	struct inode *inode;
 	struct cifs_sb_info *cifs_sb;
-#ifdef CONFIG_CIFS_DFS_UPCALL
-	int len;
-#endif
 	int rc = 0;
 
 	/* BB should we make this contingent on mount parm? */
@@ -117,15 +114,17 @@ cifs_read_super(struct super_block *sb, void *data,
 	 * complex operation (mount), and in case of fail
 	 * just exit instead of doing mount and attempting
 	 * undo it if this copy fails?*/
-	len = strlen(data);
-	cifs_sb->mountdata = kzalloc(len + 1, GFP_KERNEL);
-	if (cifs_sb->mountdata == NULL) {
-		kfree(sb->s_fs_info);
-		sb->s_fs_info = NULL;
-		return -ENOMEM;
+	if (data) {
+		int len = strlen(data);
+		cifs_sb->mountdata = kzalloc(len + 1, GFP_KERNEL);
+		if (cifs_sb->mountdata == NULL) {
+			kfree(sb->s_fs_info);
+			sb->s_fs_info = NULL;
+			return -ENOMEM;
+		}
+		strncpy(cifs_sb->mountdata, data, len + 1);
+		cifs_sb->mountdata[len] = '\0';
 	}
-	strncpy(cifs_sb->mountdata, data, len + 1);
-	cifs_sb->mountdata[len] = '\0';
 #endif
 
 	rc = cifs_mount(sb, cifs_sb, data, devname);
-- 
1.5.4.5

Re: [linux-cifs-client] [PATCH] cifs: fix oops on mount when CONFIG_CIFS_DFS_UPCALL is enabled

[Q (Igor Mammedov)]

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

A quick look trough cifs_mount and cifs_parse_mount_options functions shows
that mount will fail anyway with error EINVAL when 'data' = NULL.
May be moving NULL check at the beginning of function will be better
in this case.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-CIFS-Fix-OOPs-when-data-is-NULL.patch --]
[-- Type: text/x-patch; name=0001-CIFS-Fix-OOPs-when-data-is-NULL.patch, Size: 769 bytes --]

From 83d523d13556e98283d0fe34394819f83368efb3 Mon Sep 17 00:00:00 2001
From: q <q@q-desktop.(none)>
Date: Sun, 8 Jun 2008 08:15:07 -0400
Subject: [PATCH] [CIFS] Fix OOPs when 'data' is NULL


Signed-off-by: niallain@gmail.com <q@q-desktop.(none)>
---
 fs/cifs/cifsfs.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index e9f4ec7..aee6b9d 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -102,6 +102,9 @@ cifs_read_super(struct super_block *sb, void *data,
 #endif
 	int rc = 0;
 
+	if (!data)
+		return -EINVAL;
+
 	/* BB should we make this contingent on mount parm? */
 	sb->s_flags |= MS_NODIRATIME | MS_NOATIME;
 	sb->s_fs_info = kzalloc(sizeof(struct cifs_sb_info), GFP_KERNEL);
-- 
1.5.4.3

Re: [linux-cifs-client] [PATCH] cifs: fix oops on mount when CONFIG_CIFS_DFS_UPCALL is enabled

[Steve French]

I have not tried this code path recently and mount.cifs should always
be filling in the data field (with at least one mount option) but in a
case where the mount helper is missing, and the UNC name is of the
form //ip_address_of_server/share_name then I would prefer that we
treat this case (no mount options specified) as:
1) userid is \0 (null user)
2) null password
3) server id address comes from the beginning of the UNC name
(required or we fail)
The rest of the parms are at their defaults:
e.g. sec=ntlm  (for smb2 this will be ntlmv2, and perhaps we should
change the default for cifs as well)

On Sun, Jun 8, 2008 at 6:28 AM, Q (Igor Mammedov) <niallain@gmail.com> wrote:
> A quick look trough cifs_mount and cifs_parse_mount_options functions shows
> that mount will fail anyway with error EINVAL when 'data' = NULL.
> May be moving NULL check at the beginning of function will be better
> in this case.
>



-- 
Thanks,

Steve