r16397: Fix Klocwork #11767 and drasticly simplify the

logic in smbd/process.c. All interested (Volker,
Jerry, James etc). PLEASE REVIEW THIS CHANGE.
The logic should be identical but *much* easier
to follow and change (and shouldn't confuse Klockwork :-).
Jeremy.

6 files changed, 198 insertions, 191 deletions

diff --git a/source/smbd/message.c b/source/smbd/message.c
index fd28df0d801..31dab458443 100644
--- a/source/smbd/message.c
+++ b/source/smbd/message.c
@@ -34,211 +34,210 @@ static fstring msgfrom;
 static fstring msgto;
 
 /****************************************************************************
-deliver the message
+ Deliver the message.
 ****************************************************************************/
+
 static void msg_deliver(void)
 {
-  pstring name;
-  int i;
-  int fd;
-  char *msg;
-  int len;
-  ssize_t sz;
-
-  if (! (*lp_msg_command()))
-    {
-      DEBUG(1,("no messaging command specified\n"));
-      msgpos = 0;
-      return;
-    }
-
-  /* put it in a temporary file */
-  slprintf(name,sizeof(name)-1, "%s/msg.XXXXXX",tmpdir());
-  fd = smb_mkstemp(name);
-
-  if (fd == -1) {
-    DEBUG(1,("can't open message file %s\n",name));
-    return;
-  }
-
-  /*
-   * Incoming message is in DOS codepage format. Convert to UNIX.
-   */
+	pstring name;
+	int i;
+	int fd;
+	char *msg;
+	int len;
+	ssize_t sz;
+
+	if (! (*lp_msg_command())) {
+		DEBUG(1,("no messaging command specified\n"));
+		msgpos = 0;
+		return;
+	}
+
+	/* put it in a temporary file */
+	slprintf(name,sizeof(name)-1, "%s/msg.XXXXXX",tmpdir());
+	fd = smb_mkstemp(name);
+
+	if (fd == -1) {
+		DEBUG(1,("can't open message file %s\n",name));
+		return;
+	}
+
+	/*
+	 * Incoming message is in DOS codepage format. Convert to UNIX.
+	 */
   
-  if ((len = (int)convert_string_allocate(NULL,CH_DOS, CH_UNIX, msgbuf, msgpos, (void **)(void *)&msg, True)) < 0 || !msg) {
-    DEBUG(3,("Conversion failed, delivering message in DOS codepage format\n"));
-    for (i = 0; i < msgpos;) {
-      if (msgbuf[i] == '\r' && i < (msgpos-1) && msgbuf[i+1] == '\n') {
-	i++; continue;
-      }
-      sz = write(fd, &msgbuf[i++], 1);
-      if ( sz != 1 ) {
-	DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno ));
-      }
-    }
-  } else {
-    for (i = 0; i < len;) {
-      if (msg[i] == '\r' && i < (len-1) && msg[i+1] == '\n') {
-	i++; continue;
-      }
-      sz = write(fd, &msg[i++],1);
-      if ( sz != 1 ) {
-	DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno ));
-      }
-    }
-    SAFE_FREE(msg);
-  }
-  close(fd);
-
-
-  /* run the command */
-  if (*lp_msg_command())
-    {
-      fstring alpha_msgfrom;
-      fstring alpha_msgto;
-      pstring s;
-
-      pstrcpy(s,lp_msg_command());
-      pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,NULL,sizeof(alpha_msgfrom)));
-      pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,NULL,sizeof(alpha_msgto)));
-      standard_sub_basic(current_user_info.smb_name, s, sizeof(s));
-      pstring_sub(s,"%s",name);
-      smbrun(s,NULL);
-    }
-
-  msgpos = 0;
+	if ((len = (int)convert_string_allocate(NULL,CH_DOS, CH_UNIX, msgbuf, msgpos, (void **)(void *)&msg, True)) < 0 || !msg) {
+		DEBUG(3,("Conversion failed, delivering message in DOS codepage format\n"));
+		for (i = 0; i < msgpos;) {
+			if (msgbuf[i] == '\r' && i < (msgpos-1) && msgbuf[i+1] == '\n') {
+				i++;
+				continue;
+			}
+			sz = write(fd, &msgbuf[i++], 1);
+			if ( sz != 1 ) {
+				DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno ));
+			}
+		}
+	} else {
+		for (i = 0; i < len;) {
+			if (msg[i] == '\r' && i < (len-1) && msg[i+1] == '\n') {
+				i++;
+				continue;
+			}
+			sz = write(fd, &msg[i++],1);
+			if ( sz != 1 ) {
+				DEBUG(0,("Write error to fd %d: %ld(%d)\n",fd, (long)sz, errno ));
+			}
+		}
+		SAFE_FREE(msg);
+	}
+	close(fd);
+
+	/* run the command */
+	if (*lp_msg_command()) {
+		fstring alpha_msgfrom;
+		fstring alpha_msgto;
+		pstring s;
+
+		pstrcpy(s,lp_msg_command());
+		pstring_sub(s,"%f",alpha_strcpy(alpha_msgfrom,msgfrom,NULL,sizeof(alpha_msgfrom)));
+		pstring_sub(s,"%t",alpha_strcpy(alpha_msgto,msgto,NULL,sizeof(alpha_msgto)));
+		standard_sub_basic(current_user_info.smb_name, s, sizeof(s));
+		pstring_sub(s,"%s",name);
+		smbrun(s,NULL);
+	}
+
+	msgpos = 0;
 }
 
-
-
 /****************************************************************************
-  reply to a sends
+ Reply to a sends.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
-int reply_sends(connection_struct *conn,
-		char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
+
+int reply_sends(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
 {
-  int len;
-  char *msg;
-  int outsize = 0;
-  char *p;
+	int len;
+	char *msg;
+	int outsize = 0;
+	char *p;
 
-  START_PROFILE(SMBsends);
+	START_PROFILE(SMBsends);
 
-  msgpos = 0;
+	msgpos = 0;
 
-  if (! (*lp_msg_command())) {
-    END_PROFILE(SMBsends);
-    return(ERROR_DOS(ERRSRV,ERRmsgoff));
-  }
+	if (! (*lp_msg_command())) {
+		END_PROFILE(SMBsends);
+		return(ERROR_DOS(ERRSRV,ERRmsgoff));
+	}
 
-  outsize = set_message(outbuf,0,0,True);
+	outsize = set_message(outbuf,0,0,True);
 
-  p = smb_buf(inbuf)+1;
-  p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1;
-  p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1;
+	p = smb_buf(inbuf)+1;
+	p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1;
+	p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1;
 
-  msg = p;
+	msg = p;
 
-  len = SVAL(msg,0);
-  len = MIN(len,sizeof(msgbuf)-msgpos);
+	len = SVAL(msg,0);
+	len = MIN(len,sizeof(msgbuf)-msgpos);
 
-  memset(msgbuf,'\0',sizeof(msgbuf));
+	memset(msgbuf,'\0',sizeof(msgbuf));
 
-  memcpy(&msgbuf[msgpos],msg+2,len);
-  msgpos += len;
+	memcpy(&msgbuf[msgpos],msg+2,len);
+	msgpos += len;
 
-  msg_deliver();
+	msg_deliver();
 
-  END_PROFILE(SMBsends);
-  return(outsize);
+	END_PROFILE(SMBsends);
+	return(outsize);
 }
 
-
 /****************************************************************************
-  reply to a sendstrt
+ Reply to a sendstrt.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
-int reply_sendstrt(connection_struct *conn,
-		   char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
+
+int reply_sendstrt(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
 {
-  int outsize = 0;
-  char *p;
+	int outsize = 0;
+	char *p;
 
-  START_PROFILE(SMBsendstrt);
+	START_PROFILE(SMBsendstrt);
 
-  if (! (*lp_msg_command())) {
-    END_PROFILE(SMBsendstrt);
-    return(ERROR_DOS(ERRSRV,ERRmsgoff));
-  }
+	if (! (*lp_msg_command())) {
+		END_PROFILE(SMBsendstrt);
+		return(ERROR_DOS(ERRSRV,ERRmsgoff));
+	}
 
-  outsize = set_message(outbuf,1,0,True);
+	outsize = set_message(outbuf,1,0,True);
 
-  memset(msgbuf,'\0',sizeof(msgbuf));
-  msgpos = 0;
+	memset(msgbuf,'\0',sizeof(msgbuf));
+	msgpos = 0;
 
-  p = smb_buf(inbuf)+1;
-  p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1;
-  p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1;
+	p = smb_buf(inbuf)+1;
+	p += srvstr_pull_buf(inbuf, msgfrom, p, sizeof(msgfrom), STR_ASCII|STR_TERMINATE) + 1;
+	p += srvstr_pull_buf(inbuf, msgto, p, sizeof(msgto), STR_ASCII|STR_TERMINATE) + 1;
 
-  DEBUG( 3, ( "SMBsendstrt (from %s to %s)\n", msgfrom, msgto ) );
+	DEBUG( 3, ( "SMBsendstrt (from %s to %s)\n", msgfrom, msgto ) );
 
-  END_PROFILE(SMBsendstrt);
-  return(outsize);
+	END_PROFILE(SMBsendstrt);
+	return(outsize);
 }
 
-
 /****************************************************************************
-  reply to a sendtxt
+ Reply to a sendtxt.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
-int reply_sendtxt(connection_struct *conn,
-		  char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
+
+int reply_sendtxt(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
 {
-  int len;
-  int outsize = 0;
-  char *msg;
-  START_PROFILE(SMBsendtxt);
+	int len;
+	int outsize = 0;
+	char *msg;
+	START_PROFILE(SMBsendtxt);
 
-  if (! (*lp_msg_command())) {
-    END_PROFILE(SMBsendtxt);
-    return(ERROR_DOS(ERRSRV,ERRmsgoff));
-  }
+	if (! (*lp_msg_command())) {
+		END_PROFILE(SMBsendtxt);
+		return(ERROR_DOS(ERRSRV,ERRmsgoff));
+	}
 
-  outsize = set_message(outbuf,0,0,True);
+	outsize = set_message(outbuf,0,0,True);
 
-  msg = smb_buf(inbuf) + 1;
+	msg = smb_buf(inbuf) + 1;
 
-  len = SVAL(msg,0);
-  len = MIN(len,sizeof(msgbuf)-msgpos);
+	len = SVAL(msg,0);
+	len = MIN(len,sizeof(msgbuf)-msgpos);
 
-  memcpy(&msgbuf[msgpos],msg+2,len);
-  msgpos += len;
+	memcpy(&msgbuf[msgpos],msg+2,len);
+	msgpos += len;
 
-  DEBUG( 3, ( "SMBsendtxt\n" ) );
+	DEBUG( 3, ( "SMBsendtxt\n" ) );
 
-  END_PROFILE(SMBsendtxt);
-  return(outsize);
+	END_PROFILE(SMBsendtxt);
+	return(outsize);
 }
 
-
 /****************************************************************************
-  reply to a sendend
+ Reply to a sendend.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
-int reply_sendend(connection_struct *conn,
-		  char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
+
+int reply_sendend(connection_struct *conn, char *inbuf,char *outbuf, int dum_size, int dum_buffsize)
 {
-  int outsize = 0;
-  START_PROFILE(SMBsendend);
+	int outsize = 0;
+	START_PROFILE(SMBsendend);
 
-  if (! (*lp_msg_command())) {
-    END_PROFILE(SMBsendend);
-    return(ERROR_DOS(ERRSRV,ERRmsgoff));
-  }
+	if (! (*lp_msg_command())) {
+		END_PROFILE(SMBsendend);
+		return(ERROR_DOS(ERRSRV,ERRmsgoff));
+	}
 
-  outsize = set_message(outbuf,0,0,True);
+	outsize = set_message(outbuf,0,0,True);
 
-  DEBUG(3,("SMBsendend\n"));
+	DEBUG(3,("SMBsendend\n"));
 
-  msg_deliver();
+	msg_deliver();
 
-  END_PROFILE(SMBsendend);
-  return(outsize);
+	END_PROFILE(SMBsendend);
+	return(outsize);
 }
diff --git a/source/smbd/negprot.c b/source/smbd/negprot.c
index 5d2ed6a10d1..3347008cdf8 100644
--- a/source/smbd/negprot.c
+++ b/source/smbd/negprot.c
@@ -456,6 +456,7 @@ static const struct {
 
 /****************************************************************************
  Reply to a negprot.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_negprot(connection_struct *conn, 
diff --git a/source/smbd/nttrans.c b/source/smbd/nttrans.c
index e397139d2ef..aa6f79e1657 100644
--- a/source/smbd/nttrans.c
+++ b/source/smbd/nttrans.c
@@ -1545,6 +1545,7 @@ static int call_nt_transact_create(connection_struct *conn, char *inbuf, char *o
 
 /****************************************************************************
  Reply to a NT CANCEL request.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_ntcancel(connection_struct *conn,
diff --git a/source/smbd/process.c b/source/smbd/process.c
index 440d0ac0a50..b3ce49360d8 100644
--- a/source/smbd/process.c
+++ b/source/smbd/process.c
@@ -567,10 +567,10 @@ are used by some brain-dead clients when printing, and I don't want to
 force write permissions on print services.
 */
 #define AS_USER (1<<0)
-#define NEED_WRITE (1<<1)
+#define NEED_WRITE (1<<1) /* Must be paired with AS_USER */
 #define TIME_INIT (1<<2)
-#define CAN_IPC (1<<3)
-#define AS_GUEST (1<<5)
+#define CAN_IPC (1<<3) /* Must be paired with AS_USER */
+#define AS_GUEST (1<<5) /* Must *NOT* be paired with AS_USER */
 #define DO_CHDIR (1<<6)
 
 /* 
@@ -932,48 +932,46 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize
 			user_struct *vuser = NULL;
 
 			last_session_tag = session_tag;
-			if(session_tag != UID_FIELD_INVALID)
+			if(session_tag != UID_FIELD_INVALID) {
 				vuser = get_valid_user_struct(session_tag);           
-			if(vuser != NULL)
-				set_current_user_info(&vuser->user);
-		}
-
-		/* does this protocol need to be run as root? */
-		if (!(flags & AS_USER))
-			change_to_root_user();
-
-		/* does this protocol need a valid tree connection? */
-		if ((flags & AS_USER) && !conn) {
-			/* Amazingly, the error code depends on the command (from Samba4). */
-			if (type == SMBntcreateX) {
-				return ERROR_NT(NT_STATUS_INVALID_HANDLE);
-			} else {
-				return ERROR_DOS(ERRSRV, ERRinvnid);
+				if (vuser) {
+					set_current_user_info(&vuser->user);
+				}
 			}
 		}
 
+		/* Does this call need to be run as the connected user? */
+		if (flags & AS_USER) {
+
+			/* Does this call need a valid tree connection? */
+			if (!conn) {
+				/* Amazingly, the error code depends on the command (from Samba4). */
+				if (type == SMBntcreateX) {
+					return ERROR_NT(NT_STATUS_INVALID_HANDLE);
+				} else {
+					return ERROR_DOS(ERRSRV, ERRinvnid);
+				}
+			}
 
-		/* does this protocol need to be run as the connected user? */
-		if ((flags & AS_USER) && !change_to_user(conn,session_tag)) {
-			if (flags & AS_GUEST) 
-				flags &= ~AS_USER;
-			else
+			if (!change_to_user(conn,session_tag)) {
 				return(ERROR_FORCE_DOS(ERRSRV,ERRbaduid));
-		}
+			}
 
-		/* this code is to work around a bug is MS client 3 without
-			introducing a security hole - it needs to be able to do
-			print queue checks as guest if it isn't logged in properly */
-		if (flags & AS_USER)
-			flags &= ~AS_GUEST;
+			/* All NEED_WRITE and CAN_IPC flags must also have AS_USER. */
 
-		/* does it need write permission? */
-		if ((flags & NEED_WRITE) && !CAN_WRITE(conn))
-			return(ERROR_DOS(ERRSRV,ERRaccess));
+			/* Does it need write permission? */
+			if ((flags & NEED_WRITE) && !CAN_WRITE(conn)) {
+				return(ERROR_DOS(ERRSRV,ERRaccess));
+			}
 
-		/* ipc services are limited */
-		if (IS_IPC(conn) && (flags & AS_USER) && !(flags & CAN_IPC))
-			return(ERROR_DOS(ERRSRV,ERRaccess));	    
+			/* IPC services are limited */
+			if (IS_IPC(conn) && !(flags & CAN_IPC)) {
+				return(ERROR_DOS(ERRSRV,ERRaccess));
+			}
+		} else {
+			/* This call needs to be run as root */
+			change_to_root_user();
+		}
 
 		/* load service specific parameters */
 		if (conn) {
@@ -985,8 +983,9 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize
 
 		/* does this protocol need to be run as guest? */
 		if ((flags & AS_GUEST) && (!change_to_guest() || 
-				!check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1))))
+				!check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1)))) {
 			return(ERROR_DOS(ERRSRV,ERRaccess));
+		}
 
 		current_inbuf = inbuf; /* In case we need to defer this message in open... */
 		outsize = smb_messages[type].fn(conn, inbuf,outbuf,size,bufsize);
@@ -997,7 +996,6 @@ static int switch_message(int type,char *inbuf,char *outbuf,int size,int bufsize
 	return(outsize);
 }
 
-
 /****************************************************************************
  Construct a reply to the incoming packet.
 ****************************************************************************/
diff --git a/source/smbd/reply.c b/source/smbd/reply.c
index d333ebf32eb..e68e8662d74 100644
--- a/source/smbd/reply.c
+++ b/source/smbd/reply.c
@@ -547,6 +547,7 @@ int reply_special(char *inbuf,char *outbuf)
 
 /****************************************************************************
  Reply to a tcon.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_tcon(connection_struct *conn,
@@ -605,6 +606,7 @@ int reply_tcon(connection_struct *conn,
 
 /****************************************************************************
  Reply to a tcon and X.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_tcon_and_X(connection_struct *conn, char *inbuf,char *outbuf,int length,int bufsize)
@@ -738,6 +740,7 @@ int reply_unknown(char *inbuf,char *outbuf)
 
 /****************************************************************************
  Reply to an ioctl.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_ioctl(connection_struct *conn,
@@ -1591,6 +1594,7 @@ int reply_open_and_X(connection_struct *conn, char *inbuf,char *outbuf,int lengt
 
 /****************************************************************************
  Reply to a SMBulogoffX.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_ulogoffX(connection_struct *conn, char *inbuf,char *outbuf,int length,int bufsize)
@@ -3236,6 +3240,7 @@ int reply_flush(connection_struct *conn, char *inbuf,char *outbuf, int size, int
 
 /****************************************************************************
  Reply to a exit.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_exit(connection_struct *conn, 
@@ -3511,6 +3516,7 @@ int reply_unlock(connection_struct *conn, char *inbuf,char *outbuf, int size,
 
 /****************************************************************************
  Reply to a tdis.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_tdis(connection_struct *conn, 
@@ -3538,6 +3544,7 @@ int reply_tdis(connection_struct *conn,
 
 /****************************************************************************
  Reply to a echo.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 int reply_echo(connection_struct *conn,
diff --git a/source/smbd/sesssetup.c b/source/smbd/sesssetup.c
index 46acb20bdad..fb579707cae 100644
--- a/source/smbd/sesssetup.c
+++ b/source/smbd/sesssetup.c
@@ -635,6 +635,7 @@ static int reply_spnego_auth(connection_struct *conn, char *inbuf, char *outbuf,
 
 /****************************************************************************
  Reply to a session setup command.
+ conn POINTER CAN BE NULL HERE !
 ****************************************************************************/
 
 static int reply_sesssetup_and_X_spnego(connection_struct *conn, char *inbuf,