CVE-2016-6561: NULL Pointer Dereference (CWE-476)
value: illumos smbsrv NULL pointer dereference allows system crash.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 7.5
Published: 2017-03-31T19:59Z
Repository: https://github.com/illumos/illumos-gate
Language: C
Files changed: usr/src/uts/common/fs/smbsrv/smb2_flush.c, usr/src/uts/common/fs/smbsrv/smb_flush.c, usr/src/uts/common/fs/smbsrv/smb_ofile.c, usr/src/uts/common/smbsrv/smb_kproto.h
Commit date: 2016-10-20 20:21:54 -0400
Commit message: 7483 SMB flush on pipe triggers NULL pointer dereference in module smbsrv
Reviewed by: Gordon Ross <gwr@nexenta.com>
Reviewed by: Matt Barden <matt.barden@nexenta.com>
Reviewed by: Evan Layton <evan.layton@nexenta.com>
Reviewed by: Dan McDonald <danmcd@omniti.com>
Approved by: Gordon Ross <gwr@nexenta.com>

Diff: diff --git a/usr/src/uts/common/fs/smbsrv/smb2_flush.c b/usr/src/uts/common/fs/smbsrv/smb2_flush.c
index ecdf2fdcb8..82263d5936 100644
--- a/usr/src/uts/common/fs/smbsrv/smb2_flush.c
+++ b/usr/src/uts/common/fs/smbsrv/smb2_flush.c
@@ -9,10 +9,11 @@
  * http://www.illumos.org/license/CDDL.
  */
 
 /*
  * Copyright 2014 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2016 Syneto S.R.L. All rights reserved.
  */
 
 /*
  * Dispatch function for SMB2_FLUSH
  */
@@ -21,11 +22,10 @@
 #include <smbsrv/smb_fsops.h>
 
 smb_sdrc_t
 smb2_flush(smb_request_t *sr)
 {
-	smb_ofile_t *of = NULL;
 	uint16_t StructSize;
 	uint16_t reserved1;
 	uint32_t reserved2;
 	smb2fid_t smb2fid;
 	uint32_t status;
@@ -49,18 +49,12 @@ smb2_flush(smb_request_t *sr)
 	status = smb2sr_lookup_fid(sr, &smb2fid);
 	if (status) {
 		smb2sr_put_error(sr, status);
 		return (SDRC_SUCCESS);
 	}
-	of = sr->fid_ofile;
 
-	/*
-	 * XXX - todo:
-	 * Flush named pipe should drain writes.
-	 */
-	if ((of->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
-		(void) smb_fsop_commit(sr, of->f_cr, of->f_node);
+	smb_ofile_flush(sr, sr->fid_ofile);
 
 	/*
 	 * SMB2 Flush reply
 	 */
 	(void) smb_mbc_encodef(
diff --git a/usr/src/uts/common/fs/smbsrv/smb_flush.c b/usr/src/uts/common/fs/smbsrv/smb_flush.c
index 5de347b6b5..1d210c3d89 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_flush.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_flush.c
@@ -19,10 +19,12 @@
  * CDDL HEADER END
  */
 /*
  * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
+ *
+ * Copyright 2016 Syneto S.R.L. All rights reserved.
  */
 
 /*
  * The flush SMB is sent to ensure all data and allocation information
  * for the corresponding file has been written to stable storage. This
@@ -38,12 +40,10 @@
 
 #include <smbsrv/smb_kproto.h>
 #include <smbsrv/smb_fsops.h>
 
 
-static void smb_flush_file(struct smb_request *sr, struct smb_ofile *ofile);
-
 /*
  * smb_com_flush
  *
  * Flush any cached data for a specified file, or for all files that
  * this client has open, to stable storage. If the fid is valid (i.e.
@@ -88,39 +88,22 @@ smb_com_flush(smb_request_t *sr)
 		if (sr->fid_ofile == NULL) {
 			smbsr_error(sr, NT_STATUS_INVALID_HANDLE,
 			    ERRDOS, ERRbadfid);
 			return (SDRC_ERROR);
 		}
-
-		smb_flush_file(sr, sr->fid_ofile);
+		smb_ofile_flush(sr, sr->fid_ofile);
 	} else {
 		flist = &sr->tid_tree->t_ofile_list;
 		smb_llist_enter(flist, RW_READER);
 		file = smb_llist_head(flist);
 		while (file) {
 			mutex_enter(&file->f_mutex);
-			smb_flush_file(sr, file);
+			smb_ofile_flush(sr, file);
 			mutex_exit(&file->f_mutex);
 			file = smb_llist_next(flist, file);
 		}
 		smb_llist_exit(flist);
 	}
 
 	rc = smbsr_encode_empty_result(sr);
 	return ((rc == 0) ? SDRC_SUCCESS : SDRC_ERROR);
 }
-
-
-/*
- * smb_flush_file
- *
- * If writes on this file are not synchronous, flush it using the NFSv3
- * commit interface.
- */
-static void
-smb_flush_file(struct smb_request *sr, struct smb_ofile *ofile)
-{
-	sr->user_cr = smb_ofile_getcred(ofile);
-
-	if ((ofile->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
-		(void) smb_fsop_commit(sr, sr->user_cr, ofile->f_node);
-}
diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 34ef5dc526..582091d1dd 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -19,10 +19,11 @@
  * CDDL HEADER END
  */
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2015 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2016 Syneto S.R.L. All rights reserved.
  */
 
 /*
  * General Structures Layout
  * -------------------------
@@ -834,10 +835,31 @@ smb_ofile_seek(
 	}
 	mutex_exit(&of->f_mutex);
 	return (rc);
 }
 
+/*
+ * smb_ofile_flush
+ *
+ * If writes on this file are not synchronous, flush it using the NFSv3
+ * commit interface.
+ *
+ * XXX - todo: Flush named pipe should drain writes.
+ */
+void
+smb_ofile_flush(struct smb_request *sr, struct smb_ofile *of)
+{
+	switch (of->f_ftype) {
+	case SMB_FTYPE_DISK:
+		if ((of->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
+			(void) smb_fsop_commit(sr, of->f_cr, of->f_node);
+		break;
+	default:
+		break;
+	}
+}
+
 /*
  * smb_ofile_is_open
  */
 boolean_t
 smb_ofile_is_open(smb_ofile_t *of)
diff --git a/usr/src/uts/common/smbsrv/smb_kproto.h b/usr/src/uts/common/smbsrv/smb_kproto.h
index f394773c0a..2dd8787aeb 100644
--- a/usr/src/uts/common/smbsrv/smb_kproto.h
+++ b/usr/src/uts/common/smbsrv/smb_kproto.h
@@ -20,10 +20,11 @@
  */
 
 /*
  * Copyright (c) 2007, 2010, Oracle and/or its affiliates. All rights reserved.
  * Copyright 2015 Nexenta Systems, Inc.  All rights reserved.
+ * Copyright 2016 Syneto S.R.L.  All rights reserved.
  */
 
 /*
  * Function prototypes for the SMB module.
  */
@@ -671,10 +672,11 @@ smb_ofile_t *smb_ofile_open(smb_request_t *, smb_node_t *,
     smb_arg_open_t *, uint16_t, uint32_t, smb_error_t *);
 void smb_ofile_close(smb_ofile_t *, int32_t);
 void smb_ofile_delete(void *);
 uint32_t smb_ofile_access(smb_ofile_t *, cred_t *, uint32_t);
 int smb_ofile_seek(smb_ofile_t *, ushort_t, int32_t, uint32_t *);
+void smb_ofile_flush(smb_request_t *, smb_ofile_t *);
 boolean_t smb_ofile_hold(smb_ofile_t *);
 void smb_ofile_release(smb_ofile_t *);
 void smb_ofile_request_complete(smb_ofile_t *);
 void smb_ofile_close_all(smb_tree_t *);
 void smb_ofile_close_all_by_pid(smb_tree_t *, uint16_t);

Vulnerable code: 	smb_ofile_t *of = NULL;
	of = sr->fid_ofile;
	/*
	 * XXX - todo:
	 * Flush named pipe should drain writes.
	 */
	if ((of->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
		(void) smb_fsop_commit(sr, of->f_cr, of->f_node);
static void smb_flush_file(struct smb_request *sr, struct smb_ofile *ofile);


		smb_flush_file(sr, sr->fid_ofile);
			smb_flush_file(sr, file);


/*
 * smb_flush_file
 *
 * If writes on this file are not synchronous, flush it using the NFSv3
 * commit interface.
 */
static void
smb_flush_file(struct smb_request *sr, struct smb_ofile *ofile)
{
	sr->user_cr = smb_ofile_getcred(ofile);

	if ((ofile->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
		(void) smb_fsop_commit(sr, sr->user_cr, ofile->f_node);
}
Fixed code:  * Copyright 2016 Syneto S.R.L. All rights reserved.
	smb_ofile_flush(sr, sr->fid_ofile);
 *
 * Copyright 2016 Syneto S.R.L. All rights reserved.
		smb_ofile_flush(sr, sr->fid_ofile);
			smb_ofile_flush(sr, file);
 * Copyright 2016 Syneto S.R.L. All rights reserved.
/*
 * smb_ofile_flush
 *
 * If writes on this file are not synchronous, flush it using the NFSv3
 * commit interface.
 *
 * XXX - todo: Flush named pipe should drain writes.
 */
void
smb_ofile_flush(struct smb_request *sr, struct smb_ofile *of)
{
	switch (of->f_ftype) {
	case SMB_FTYPE_DISK:
		if ((of->f_node->flags & NODE_FLAGS_WRITE_THROUGH) == 0)
			(void) smb_fsop_commit(sr, of->f_cr, of->f_node);
		break;
	default:
		break;
	}
}

 * Copyright 2016 Syneto S.R.L.  All rights reserved.
void smb_ofile_flush(smb_request_t *, smb_ofile_t *);