Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Hawkins Jiawei <yin31149@gmail.com>
To:     yin31149@gmail.com, Steve French <sfrench@samba.org>,
        Paulo Alcantara <pc@cjr.nz>,
        Ronnie Sahlberg <lsahlber@redhat.com>,
        Shyam Prasad N <sprasad@microsoft.com>,
        Tom Talpey <tom@talpey.com>
Cc:     18801353760@163.com, linux-kernel@vger.kernel.org,
        linux-fsdevel@vger.kernel.org, linux-cifs@vger.kernel.org,
        samba-technical@lists.samba.org
Subject: [PATCH -next 1/5] smb3: fix possible null-ptr-deref when parsing param
Date:   Mon, 24 Oct 2022 00:39:43 +0800
Message-Id: <20221023163945.39920-2-yin31149@gmail.com>
In-Reply-To: <20221023163945.39920-1-yin31149@gmail.com>
References: <20221023163945.39920-1-yin31149@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

According to commit "vfs: parse: deal with zero length string value",
kernel will set the param->string to null pointer in vfs_parse_fs_string()
if fs string has zero length.

Yet the problem is that, smb3_fs_context_parse_param() will dereferences
the param->string, without checking whether it is a null pointer, which
may trigger a null-ptr-deref bug.

This patch solves it by adding sanity check on param->string
in smb3_fs_context_parse_param().

Signed-off-by: Hawkins Jiawei <yin31149@gmail.com>
---
 fs/cifs/fs_context.c | 58 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/fs_context.c b/fs/cifs/fs_context.c
index 45119597c765..7832e5d6bbb0 100644
--- a/fs/cifs/fs_context.c
+++ b/fs/cifs/fs_context.c
@@ -858,7 +858,8 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 	 * fs_parse can not handle string options with an empty value so
 	 * we will need special handling of them.
 	 */
-	if (param->type == fs_value_is_string && param->string[0] == 0) {
+	if ((param->type == fs_value_is_string && param->string[0] == 0) ||
+	    param->type == fs_value_is_empty) {
 		if (!strcmp("pass", param->key) || !strcmp("password", param->key)) {
 			skip_parsing = true;
 			opt = Opt_pass;
@@ -1124,6 +1125,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 	case Opt_source:
 		kfree(ctx->UNC);
 		ctx->UNC = NULL;
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		switch (smb3_parse_devname(param->string, ctx)) {
 		case 0:
 			break;
@@ -1181,6 +1187,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		}
 		break;
 	case Opt_ip:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (strlen(param->string) == 0) {
 			ctx->got_ip = false;
 			break;
@@ -1194,6 +1205,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		ctx->got_ip = true;
 		break;
 	case Opt_domain:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (strnlen(param->string, CIFS_MAX_DOMAINNAME_LEN)
 				== CIFS_MAX_DOMAINNAME_LEN) {
 			pr_warn("domain name too long\n");
@@ -1209,6 +1225,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		cifs_dbg(FYI, "Domain name set\n");
 		break;
 	case Opt_srcaddr:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (!cifs_convert_address(
 				(struct sockaddr *)&ctx->srcaddr,
 				param->string, strlen(param->string))) {
@@ -1218,6 +1239,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		}
 		break;
 	case Opt_iocharset:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (strnlen(param->string, 1024) >= 65) {
 			pr_warn("iocharset name too long\n");
 			goto cifs_parse_mount_err;
@@ -1237,6 +1263,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		cifs_dbg(FYI, "iocharset set to %s\n", ctx->iocharset);
 		break;
 	case Opt_netbiosname:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		memset(ctx->source_rfc1001_name, 0x20,
 			RFC1001_NAME_LEN);
 		/*
@@ -1257,6 +1288,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 			pr_warn("netbiosname longer than 15 truncated\n");
 		break;
 	case Opt_servern:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		/* last byte, type, is 0x20 for servr type */
 		memset(ctx->target_rfc1001_name, 0x20,
 			RFC1001_NAME_LEN_WITH_NULL);
@@ -1277,6 +1313,11 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 			pr_warn("server netbiosname longer than 15 truncated\n");
 		break;
 	case Opt_ver:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		/* version of mount userspace tools, not dialect */
 		/* If interface changes in mount.cifs bump to new ver */
 		if (strncasecmp(param->string, "1", 1) == 0) {
@@ -1292,16 +1333,31 @@ static int smb3_fs_context_parse_param(struct fs_context *fc,
 		pr_warn("Invalid mount helper version specified\n");
 		goto cifs_parse_mount_err;
 	case Opt_vers:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		/* protocol version (dialect) */
 		if (cifs_parse_smb_version(fc, param->string, ctx, is_smb3) != 0)
 			goto cifs_parse_mount_err;
 		ctx->got_version = true;
 		break;
 	case Opt_sec:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (cifs_parse_security_flavors(fc, param->string, ctx) != 0)
 			goto cifs_parse_mount_err;
 		break;
 	case Opt_cache:
+		if (!param->string) {
+			cifs_errorf(fc, "Bad value '(null)' for mount option '%s'\n",
+				    param->key);
+			goto cifs_parse_mount_err;
+		}
 		if (cifs_parse_cache_flavor(fc, param->string, ctx) != 0)
 			goto cifs_parse_mount_err;
 		break;
-- 
2.25.1