Merge pull request #27 from ArvidNorr/deadlock

Deadlock

5 files changed, 114 insertions, 23 deletions

diff --git a/libxrdp/libxrdp.c b/libxrdp/libxrdp.c
index 5c228305..cafd63fb 100644
--- a/libxrdp/libxrdp.c
+++ b/libxrdp/libxrdp.c
@@ -75,10 +75,12 @@ libxrdp_process_data(struct xrdp_session* session)
   int rv;
   int code;
   int term;
+  int dead_lock_counter;
 
   term = 0;
   cont = 1;
   rv = 0;
+  dead_lock_counter  = 0 ;
   while ((cont || !session->up_and_running) && !term)
   {
     if (session->is_term != 0)
@@ -102,6 +104,7 @@ libxrdp_process_data(struct xrdp_session* session)
         session->up_and_running = 0;
         break;
       case 0:
+        dead_lock_counter ++ ;  
         break;
       case RDP_PDU_CONFIRM_ACTIVE: /* 3 */
         xrdp_rdp_process_confirm_active((struct xrdp_rdp*)session->rdp,
@@ -118,7 +121,16 @@ libxrdp_process_data(struct xrdp_session* session)
         break;
       default:
         g_writeln("unknown in libxrdp_process_data");
+        dead_lock_counter ++ ;
         break;
+    }    
+    if(dead_lock_counter>100000)
+    {
+      /*This situation can happen and this is a workaround*/	
+      cont = 0 ;
+      g_writeln("Serious programming error we were locked in a deadly loop") ;
+      g_writeln("remaining :%d",session->s->end-session->s->next_packet);
+      session->s->next_packet = 0;
     }
     if (cont)
     {
@@ -616,7 +628,10 @@ libxrdp_reset(struct xrdp_session* session,
   }
   /* process till up and running */
   session->up_and_running = 0;
-  libxrdp_process_data(session);
+  if(libxrdp_process_data(session)!=0)
+  {
+    g_writeln("non handled error from libxrdp_process_data");    
+  }
   return 0;
 }
 
@@ -658,6 +673,11 @@ libxrdp_query_channel(struct xrdp_session* session, int index,
 
   rdp = (struct xrdp_rdp*)session->rdp;
   mcs = rdp->sec_layer->mcs_layer;
+  if(mcs->channel_list==NULL)
+  {
+    g_writeln("libxrdp_query_channel - No channel initialized");
+    return 1 ;
+  }
   count = mcs->channel_list->count;
   if (index < 0 || index >= count)
   {
@@ -668,6 +688,7 @@ libxrdp_query_channel(struct xrdp_session* session, int index,
   if (channel_item == 0)
   {
     /* this should not happen */
+    g_writeln("libxrdp_query_channel - channel item is 0"); 
     return 1;
   }
   if (channel_name != 0)
@@ -695,6 +716,11 @@ libxrdp_get_channel_id(struct xrdp_session* session, char* name)
 
   rdp = (struct xrdp_rdp*)session->rdp;
   mcs = rdp->sec_layer->mcs_layer;
+  if(mcs->channel_list==NULL)
+  {
+    g_writeln("libxrdp_get_channel_id No channel initialized");
+    return -1 ;
+  }
   count = mcs->channel_list->count;
   for (index = 0; index < count; index++)
   {
@@ -737,6 +763,7 @@ libxrdp_send_to_channel(struct xrdp_session* session, int channel_id,
   s_mark_end(s);
   if (xrdp_channel_send(chan, s, channel_id, total_data_len, flags) != 0)
   {
+    g_writeln("Debug - data NOT sent to channel");  
     free_stream(s);
     return 1;
   }
diff --git a/libxrdp/xrdp_channel.c b/libxrdp/xrdp_channel.c
index 00497c35..092cefa4 100644
--- a/libxrdp/xrdp_channel.c
+++ b/libxrdp/xrdp_channel.c
@@ -35,7 +35,11 @@ static struct mcs_channel_item* APP_CC
 xrdp_channel_get_item(struct xrdp_channel* self, int channel_id)
 {
   struct mcs_channel_item* channel;
-
+  if(self->mcs_layer->channel_list==NULL)
+  {
+    g_writeln("xrdp_channel_get_item - No channel initialized");
+    return NULL ;
+  }
   channel = (struct mcs_channel_item*)
                list_get_item(self->mcs_layer->channel_list, channel_id);
   return channel;
@@ -62,6 +66,7 @@ xrdp_channel_delete(struct xrdp_channel* self)
   {
     return;
   }
+  g_memset(self,0,sizeof(struct xrdp_channel));
   g_free(self);
 }
 
@@ -88,8 +93,9 @@ xrdp_channel_send(struct xrdp_channel* self, struct stream* s, int channel_id,
   struct mcs_channel_item* channel;
 
   channel = xrdp_channel_get_item(self, channel_id);
-  if (channel == 0)
+  if (channel == NULL)
   {
+    g_writeln("xrdp_channel_send - no such channel");  
     return 1;
   }
   s_pop_layer(s, channel_hdr);
@@ -101,6 +107,7 @@ xrdp_channel_send(struct xrdp_channel* self, struct stream* s, int channel_id,
   out_uint32_le(s, flags);
   if (xrdp_sec_send(self->sec_layer, s, channel->chanid) != 0)
   {
+    g_writeln("xrdp_channel_send - failure sending data");  
     return 1;
   }
   return 0;
@@ -159,13 +166,14 @@ xrdp_channel_process(struct xrdp_channel* self, struct stream* s,
   int channel_id;
   struct mcs_channel_item* channel;
 
+
   /* this assumes that the channels are in order of chanid(mcs channel id)
      but they should be, see xrdp_sec_process_mcs_data_channels
      the first channel should be MCS_GLOBAL_CHANNEL + 1, second
      one should be MCS_GLOBAL_CHANNEL + 2, and so on */
   channel_id = (chanid - MCS_GLOBAL_CHANNEL) - 1;
   channel = xrdp_channel_get_item(self, channel_id);
-  if (channel == 0)
+  if (channel == NULL)
   {
     g_writeln("xrdp_channel_process, channel not found");
     return 1;
diff --git a/libxrdp/xrdp_mcs.c b/libxrdp/xrdp_mcs.c
index af65faf2..1a7a0411 100644
--- a/libxrdp/xrdp_mcs.c
+++ b/libxrdp/xrdp_mcs.c
@@ -65,11 +65,15 @@ xrdp_mcs_delete(struct xrdp_mcs* self)
   }
   list_delete(self->channel_list);
   xrdp_iso_delete(self->iso_layer);
+  /* make sure we get null pointer exception if struct is used again. */
+  DEBUG(("xrdp_mcs_delete processed"))
+  g_memset(self,0,sizeof(struct xrdp_mcs)) ; 
   g_free(self);
 }
 
 /*****************************************************************************/
-/* returns error */
+/* This function sends channel join confirm*/
+/* returns error = 1 ok = 0*/
 static int APP_CC
 xrdp_mcs_send_cjcf(struct xrdp_mcs* self, int userid, int chanid)
 {
@@ -87,7 +91,7 @@ xrdp_mcs_send_cjcf(struct xrdp_mcs* self, int userid, int chanid)
   out_uint8(s, (MCS_CJCF << 2) | 2);
   out_uint8(s, 0);
   out_uint16_be(s, userid);
-  out_uint16_be(s, chanid);
+  out_uint16_be(s, chanid); /* TODO Explain why we send this two times */
   out_uint16_be(s, chanid);
   s_mark_end(s);
   if (xrdp_iso_send(self->iso_layer, s) != 0)
@@ -122,20 +126,33 @@ xrdp_mcs_recv(struct xrdp_mcs* self, struct stream* s, int* chan)
     }
     in_uint8(s, opcode);
     appid = opcode >> 2;
-    if (appid == MCS_DPUM)
+    if (appid == MCS_DPUM) /* Disconnect Provider Ultimatum */
     {
+      g_writeln("received Disconnect Provider Ultimatum");
       DEBUG(("  out xrdp_mcs_recv appid != MCS_DPUM"));
       return 1;
     }
     /* this is channels getting added from the client */
     if (appid == MCS_CJRQ)
     {
+      g_writeln("channel join request received");	
       in_uint16_be(s, userid);
       in_uint16_be(s, chanid);
-      DEBUG(("  adding channel %4.4x", chanid));
-      xrdp_mcs_send_cjcf(self, userid, chanid);
+      DEBUG(("xrdp_mcs_recv  adding channel %4.4x", chanid));
+      if(xrdp_mcs_send_cjcf(self, userid, chanid)!=0)
+      {
+        g_writeln("Non handled error from xrdp_mcs_send_cjcf") ;
+      }
       continue;
     }
+    if(appid==MCS_SDRQ || appid==MCS_SDIN)
+    {
+      break ;
+    }
+    else
+    {
+      g_writeln("Recieved an unhandled appid:%d",appid);
+    }
     break;
   }
   if (appid != MCS_SDRQ)
diff --git a/libxrdp/xrdp_rdp.c b/libxrdp/xrdp_rdp.c
index c9f5dedf..ba777092 100644
--- a/libxrdp/xrdp_rdp.c
+++ b/libxrdp/xrdp_rdp.c
@@ -113,13 +113,25 @@ xrdp_rdp_read_config(struct xrdp_client_info* client_info)
       {
         client_info->crypt_level = 3;
       }
+      else
+      {
+        g_writeln("Warning: Your configured crypt level is"
+		  "undefined 'high' will be used");
+        client_info->crypt_level = 3;  
+      }	  
     }
     else if (g_strcasecmp(item, "channel_code") == 0)
     {
-      if (g_strcasecmp(value, "1") == 0)
+      if ((g_strcasecmp(value, "yes") == 0) ||
+          (g_strcasecmp(value, "1") == 0) ||
+          (g_strcasecmp(value, "true") == 0))      
       {
         client_info->channel_code = 1;
       }
+      else
+      {
+        g_writeln("Info: All channels are disabled");
+      }
     }
     else if (g_strcasecmp(item, "max_bpp") == 0)
     {
@@ -223,7 +235,7 @@ xrdp_rdp_recv(struct xrdp_rdp* self, struct stream* s, int* code)
     {
       s->next_packet = 0;
       *code = -1;
-      DEBUG(("out xrdp_rdp_recv"));
+      DEBUG(("out (1) xrdp_rdp_recv"));
       return 0;
     }
     if (error != 0)
@@ -235,35 +247,47 @@ xrdp_rdp_recv(struct xrdp_rdp* self, struct stream* s, int* code)
     {
       if (chan > MCS_GLOBAL_CHANNEL)
       {
-        xrdp_channel_process(self->sec_layer->chan_layer, s, chan);
+        if(xrdp_channel_process(self->sec_layer->chan_layer, s, chan)!=0)
+        {
+          g_writeln("xrdp_channel_process returned unhandled error") ;
+        }
+      }
+      else
+      {
+        g_writeln("Wrong channel Id to be handled by xrdp_channel_process %d",chan);
       }
       s->next_packet = 0;
       *code = 0;
-      DEBUG(("out xrdp_rdp_recv"));
+      DEBUG(("out (2) xrdp_rdp_recv"));
       return 0;
     }
     s->next_packet = s->p;
   }
   else
   {
+    DEBUG(("xrdp_rdp_recv stream not touched"))
     s->p = s->next_packet;
   }
   if (!s_check_rem(s, 6))
   {
     s->next_packet = 0;
     *code = 0;
-    DEBUG(("out xrdp_rdp_recv"));
+    DEBUG(("out (3) xrdp_rdp_recv"));
     len = (int)(s->end - s->p);
     g_writeln("xrdp_rdp_recv: bad RDP packet, length [%d]", len);
     return 0;
   }
-  in_uint16_le(s, len);
-  in_uint16_le(s, pdu_code);
-  *code = pdu_code & 0xf;
-  in_uint8s(s, 2); /* mcs user id */
-  s->next_packet += len;
-  DEBUG(("out xrdp_rdp_recv"));
-  return 0;
+  else
+  {
+    in_uint16_le(s, len);
+    /*g_writeln("New len received : %d next packet: %d s_end: %d",len,s->next_packet,s->end); */
+    in_uint16_le(s, pdu_code);
+    *code = pdu_code & 0xf;
+    in_uint8s(s, 2); /* mcs user id */
+    s->next_packet += len;
+    DEBUG(("out (4) xrdp_rdp_recv"));
+    return 0;
+  }
 }
 
 /*****************************************************************************/
diff --git a/libxrdp/xrdp_sec.c b/libxrdp/xrdp_sec.c
index 952ff12f..381e8435 100644
--- a/libxrdp/xrdp_sec.c
+++ b/libxrdp/xrdp_sec.c
@@ -148,9 +148,20 @@ xrdp_sec_create(struct xrdp_rdp* owner, struct trans* trans, int crypt_level,
       self->rc4_key_size = 2;
       self->crypt_level = 3;
       break;
+    default:
+      g_writeln("Fatal : Illegal crypt_level");
+      break ;
   }
   self->channel_code = channel_code;
+  if(self->decrypt_rc4_info!=NULL)
+  {
+    g_writeln("xrdp_sec_create - decrypt_rc4_info already created !!!");
+  }
   self->decrypt_rc4_info = ssl_rc4_info_create();
+  if(self->encrypt_rc4_info!=NULL)
+  {
+    g_writeln("xrdp_sec_create - encrypt_rc4_info already created !!!");
+  }
   self->encrypt_rc4_info = ssl_rc4_info_create();
   self->mcs_layer = xrdp_mcs_create(self, trans, &self->client_mcs_data,
                                     &self->server_mcs_data);
@@ -165,14 +176,17 @@ xrdp_sec_delete(struct xrdp_sec* self)
 {
   if (self == 0)
   {
+    g_writeln("xrdp_sec_delete:  indata is null");  
     return;
   }
   xrdp_channel_delete(self->chan_layer);
   xrdp_mcs_delete(self->mcs_layer);
-  ssl_rc4_info_delete(self->decrypt_rc4_info);
-  ssl_rc4_info_delete(self->encrypt_rc4_info);
+  ssl_rc4_info_delete(self->decrypt_rc4_info); /* TODO clear all data */
+  ssl_rc4_info_delete(self->encrypt_rc4_info); /* TODO clear all data */
   g_free(self->client_mcs_data.data);
   g_free(self->server_mcs_data.data);
+  /* Crypto information must always be cleared */
+  g_memset(self,0,sizeof(struct xrdp_sec));
   g_free(self);
 }
 
@@ -722,6 +736,7 @@ xrdp_sec_process_mcs_data_channels(struct xrdp_sec* self, struct stream* s)
   /* this is an option set in xrdp.ini */
   if (self->channel_code != 1) /* are channels on? */
   {
+    g_writeln("Processing channel data from client - The channel is off");  
     return 0;
   }
   in_uint32_le(s, num_channels);