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.

git-svn-id: svn+ssh://svn.samba.org/data/svn/samba/branches/SAMBA_3_0@16397 0c0555d6-39d7-0310-84fc-f1cc0bd64818

diff --git a/source/smbd/message.c b/source/smbd/message.c
index fd28df0d801fa2aeab2c715d8a20d7f5d45f82dd..31dab458443cb24b670f65bcb507d4037aae74d6 100644 (file)
--- 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 5d2ed6a10d14ccd212076a477b00abbc5e65d0f3..3347008cdf8a523b0e46b39d05d7e59edb3aff04 100644 (file)
--- 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 e397139d2efe443da2af89420e7835a9ceff23d7..aa6f79e16571c0c4829b46a8bca5aa04cb913478 100644 (file)
--- 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 440d0ac0a50505fb4cc177c712286add09b73b07..b3ce49360d836ef438258c2d912de69f00e22183 100644 (file)
--- 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 d333ebf32eb4985aba3a08f720b54ef58ccb990c..e68e8662d74f2a236e3851f104e6e10cd67eb103 100644 (file)
--- 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 46acb20bdadce44dccf386cd357684e03bb5dec4..fb579707caeb24ab0d2e024bbfc68567efd9928d 100644 (file)
--- 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,