[Spread-cvs] commit: r669 - trunk/daemon

jschultz at spread.org jschultz at spread.org
Tue Jan 21 21:40:17 EST 2014


Author: jschultz
Date: 2014-01-21 21:40:17 -0500 (Tue, 21 Jan 2014)
New Revision: 669

Modified:
   trunk/daemon/membership.c
Log:
Adding bounds checks to Create_form1, Fill_form1 and Read_form2.  Plus some cleanup.


Modified: trunk/daemon/membership.c
===================================================================
--- trunk/daemon/membership.c	2014-01-21 23:36:16 UTC (rev 668)
+++ trunk/daemon/membership.c	2014-01-22 02:40:17 UTC (rev 669)
@@ -1439,19 +1439,24 @@
 	Alarmp(SPLOG_INFO, MEMB, "Create_form1: putting Aru = %d and Highest_Seq = %d on rg_info form1 token\n", Aru, Highest_seq);
 
 	/* update holes */
-	rg_info->num_holes	= 0;
+
+	rg_info->num_holes = 0;
+
 	for( index = My_aru+1; index <= Highest_seq; index++ )
 	{
 	    pack_entry = index & PACKET_MASK;
 	    if( ! Packets[pack_entry].exist )
 	    {
+		num_bytes += sizeof(int32);
+		rg_info->num_holes++;
+
+		if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		    Alarmp( SPLOG_FATAL, MEMB, "Create_form1:%d: token too big; num_bytes (%d); too many holes (%d)?\n", __LINE__, num_bytes, rg_info->num_holes );
+		}
+
+		Alarmp( SPLOG_INFO, MEMB, "INSERT HOLE 1 IS %d My_aru is %d, Highest_seq is %d\n", index, My_aru, Highest_seq );
 		*holes_procs_ptr = index;
-		Alarm( MEMB ,
-		    "INSERT HOLE 1 IS %d My_aru is %d, Highest_seq is %d\n",
-		    index,My_aru, Highest_seq);
 		holes_procs_ptr++;
-		num_bytes += sizeof(int32);
-		rg_info->num_holes++;
 	    }
 	}
 
@@ -1460,8 +1465,13 @@
 	/* insert self in trans and commit */
 	rg_info->num_commit	= 1;
 	rg_info->num_trans	= 1;
+	num_bytes += sizeof(int32);
+
+	if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+	    Alarmp( SPLOG_FATAL, MEMB, "Create_form1:%d: token too big; num_bytes (%d); too many holes (%d)?\n", __LINE__, num_bytes, rg_info->num_holes );
+	}
+
 	*holes_procs_ptr = My.id;
-	num_bytes += sizeof(int32);
 	holes_procs_ptr++;
 
 	/* insert other members of commit set */
@@ -1471,10 +1481,16 @@
 		if( Commit_set.members[i] == My.id ) continue;
 
 		/* insert this member */
+		num_bytes += sizeof(int32);
+		rg_info->num_commit++;
+
+		if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		    Alarmp( SPLOG_FATAL, MEMB, "Create_form1:%d: token too big; num_bytes (%d); too many holes (%d), too many num_commit (%d)?\n", 
+			    __LINE__, num_bytes, rg_info->num_holes, rg_info->num_commit );
+		}
+
 		*holes_procs_ptr = Commit_set.members[i];
 		holes_procs_ptr++;
-		num_bytes += sizeof(int32);
-		rg_info->num_commit++;
 	}
 	
 	send_scat.num_elements = 4;
@@ -1487,8 +1503,7 @@
 	send_scat.elements[3].buf = rg_info_buf;
 	send_scat.elements[3].len = num_bytes;
 
-	form_token.rtr_len = send_scat.elements[1].len + send_scat.elements[2].len + 
-			      send_scat.elements[3].len;
+	form_token.rtr_len = send_scat.elements[1].len + send_scat.elements[2].len + send_scat.elements[3].len;
 
 	/* compute whom to send to */
 	if( F_members.num_pending > 0 )
@@ -1663,13 +1678,13 @@
 			i = m_info->num_members;
 			for( j=0; j < Membership.num_segments; j++ )
 			    for( k=0; k < Membership.segments[j].num_procs; k++, i++ )
-				m_info->members[i] = 
-					Membership.segments[j].procs[k]->id;
+				m_info->members[i] = Membership.segments[j].procs[k]->id;
+
 			m_info->num_pending = i - m_info->num_members -1;
 			m_info->num_members += 1;
-		}else Alarm( EXIT, "Fill_form1: invalid rep type: %d\n",
-			r_info->reps[r_info->rep_index].type );
 
+		}else Alarm( EXIT, "Fill_form1: invalid rep type: %d\n", r_info->reps[r_info->rep_index].type );
+
 		r_info->rep_index++;
 
 	}else Alarm( EXIT, "Fill_form1: invalid State: %d\n",State );
@@ -1699,20 +1714,26 @@
 				     is too subtle for it) */
 	for( i=0; i < *old_num_rings; i++ )
 	{
-	    bytes_to_copy = sizeof(ring_info) +
-			( old_rg_info->num_holes + old_rg_info->num_commit )* sizeof(int32);
+	    bytes_to_copy = sizeof(ring_info) + ( old_rg_info->num_holes + old_rg_info->num_commit )* sizeof(int32);
+
 	    if( Memb_is_equal( old_rg_info->memb_id, Membership_id ) )
 	    {
 		my_rg_info = old_rg_info;
 		c_ptr = (char *) old_rg_info;
 		my_holes_procs_ptr = (int32 *)&c_ptr[sizeof(ring_info)];
                 old_rg_info = (ring_info *)&c_ptr[bytes_to_copy];
-	    }else{
+
+	    } else {
 		new_rg_info= (ring_info    *)&rg_info_buf[num_bytes];
+		num_bytes += bytes_to_copy;
+
+		if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+		}
+
 		memmove((char *)new_rg_info, (char *)old_rg_info, bytes_to_copy );
 		c_ptr = (char *) old_rg_info;
 		old_rg_info = (ring_info *)&c_ptr[bytes_to_copy];
-		num_bytes   += bytes_to_copy;
 		(*new_num_rings)++;
 	    }
 	}
@@ -1721,6 +1742,10 @@
 	num_bytes += sizeof(ring_info);
 	(*new_num_rings)++;
 
+	if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+	    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+	}
+
 	new_rg_info->memb_id	 = Membership_id;
         new_rg_info->trans_time  = 0;
 	new_rg_info->num_holes	 = 0;
@@ -1742,7 +1767,10 @@
                  * to remove ourselves from the m_info list and not
                  * create this ring_info 
                  */
-                Alarmp( SPLOG_WARNING, MEMB, "Fill_form1: ring_info entry for %u.%u.%u.%u will not fit in FORM token. Removing self from current membership attempt by removing IP from m_info list\n", IP1(My.id), IP2(My.id), IP3(My.id), IP4(My.id));
+                Alarmp( SPLOG_WARNING, MEMB, 
+			"Fill_form1: ring_info entry for %u.%u.%u.%u will not fit in FORM token. Removing self from current membership attempt and m_info list\n", 
+			IP1(My.id), IP2(My.id), IP3(My.id), IP4(My.id));
+
                 /* since new ring is always at the end, we just decrease current byte count */
                 num_bytes = num_bytes - sizeof(ring_info);
                 (*new_num_rings)--;
@@ -1766,11 +1794,16 @@
                     pack_entry = index & PACKET_MASK;
                     if( ! Packets[pack_entry].exist )
                     {
+			num_bytes += sizeof(int32);
+			new_rg_info->num_holes++;
+
+			if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+			    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+			}
+
+			Alarmp( SPLOG_INFO, MEMB , "INSERT HOLE 2 IS %d\n", index);
 			*new_holes_procs_ptr = index;
-			Alarm( MEMB , "INSERT HOLE 2 IS %d\n",index);
 			new_holes_procs_ptr++;
-			num_bytes     += sizeof(int32);
-			new_rg_info->num_holes++;
                     }
                 }
 
@@ -1779,9 +1812,14 @@
                 /* insert self in trans and commit */
                 new_rg_info->num_commit	= 1;
                 new_rg_info->num_trans	= 1;
+                num_bytes += sizeof(int32);
+
+		if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+		}
+
                 *new_holes_procs_ptr = My.id;
                 new_holes_procs_ptr++;
-                num_bytes += sizeof(int32);
 
                 /* insert other members of commit set */
                 for( i=0; i < Commit_set.num_members; i++ )
@@ -1790,18 +1828,23 @@
                     if( Commit_set.members[i] == My.id ) continue;
 
                     /* insert this member */
+                    num_bytes += sizeof(int32);
+                    new_rg_info->num_commit++;
+
+		    if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		        Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+		    }
+
                     *new_holes_procs_ptr = Commit_set.members[i];
                     new_holes_procs_ptr++;
-                    num_bytes += sizeof(int32);
-                    new_rg_info->num_commit++;
                 }
             }
 	}else{
 	
 	    members_info temp_set;
 
-	    if( my_rg_info->aru         > Aru ) {
-		new_rg_info->aru 	= my_rg_info->aru;
+	    if( my_rg_info->aru > Aru ) {
+		new_rg_info->aru = my_rg_info->aru;
 		Alarmp( SPLOG_INFO, MEMB, "my_rg_info->aru (%d) > Aru (%d) -> setting new_rg_info->aru to my_rg_info\n", my_rg_info->aru, Aru );
 	    }
 
@@ -1815,31 +1858,41 @@
 		pack_entry = *my_holes_procs_ptr & PACKET_MASK;
 		if( ! Packets[pack_entry].exist )
 		{
-			*new_holes_procs_ptr	= *my_holes_procs_ptr;
+			num_bytes += sizeof(int32);
+			new_rg_info->num_holes++;
+
+			if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+  			    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+			}
+
 			Alarmp( SPLOG_INFO, MEMB, "INSERT HOLE 3 IS %d, My_aru is %d, Highest_seq is %d\n", *new_holes_procs_ptr, My_aru, Highest_seq );
+			*new_holes_procs_ptr = *my_holes_procs_ptr;
 			new_holes_procs_ptr++;
-			num_bytes	+= sizeof(int32);
-			new_rg_info->num_holes++;
 		}
 		my_holes_procs_ptr++;
 	    }
 
 	    if( my_rg_info->highest_seq < Highest_seq )
             {
-		for( index = my_rg_info->highest_seq+1; 
-			index <= Highest_seq; index++ )
+		for( index = my_rg_info->highest_seq+1; index <= Highest_seq; index++ )
 		{
 		    pack_entry = index & PACKET_MASK;
 		    if( ! Packets[pack_entry].exist )
 		    {
+			num_bytes += sizeof(int32);
+			new_rg_info->num_holes++;
+
+			if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+  			    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+			}
+
 			Alarm( MEMB , "INSERT HOLE 4 IS %d\n",index);
 			*new_holes_procs_ptr = index;
 			new_holes_procs_ptr++;
-			num_bytes     += sizeof(int32);
-			new_rg_info->num_holes++;
 		    }
 		} 
             }
+
 	    /* setting temp_set to be trans members only */
 	    temp_set.num_members = 0;
 	    for( i = 0; i < my_rg_info->num_trans; i++ )
@@ -1874,9 +1927,14 @@
 	    new_rg_info->num_trans  = temp_set.num_pending;
 	    for( i = 0; i < temp_set.num_members; i++ )
 	    {
+		num_bytes += sizeof(int32);
+
+		if ( num_bytes > (int) sizeof( rg_info_buf ) ) {
+		    Alarmp( SPLOG_FATAL, MEMB, "Fill_form1:%d: token too big; num_bytes (%d)\n", __LINE__, num_bytes );
+		}
+
 		*new_holes_procs_ptr = temp_set.members[i];
 		new_holes_procs_ptr++;
-		num_bytes += sizeof(int32);
 	    }
 	}
 
@@ -1890,8 +1948,7 @@
 	send_scat.elements[3].buf = rg_info_buf;
 	send_scat.elements[3].len = num_bytes;
 
-	form_token->rtr_len = send_scat.elements[1].len + send_scat.elements[2].len + 
-			      send_scat.elements[3].len;
+	form_token->rtr_len = send_scat.elements[1].len + send_scat.elements[2].len + send_scat.elements[3].len;
 
 	/* compute whom to send to */
 	if( m_info->num_pending > 0 )
@@ -1953,20 +2010,23 @@
 	char		*rings_buf;
 	int		num_bytes;
 	int		bytes_to_skip;
+	int             tot_len;
 	proc		p;
 	int		ret;
 	int		i;
         int32           memb_time = 0;
 
-	if ( Alarm_get_priority() >= SPLOG_INFO && ( Alarm_get_types() & MEMB ) != 0 ) {
-		Alarmp( SPLOG_INFO, MEMB, "Read_form2: RECEIVED following token:\n" );
-	        Memb_print_form_token( scat );
-	}
-	
 	num_bytes  = 0;
 
 	form_token = (token_header *)scat->elements[0].buf;
 
+	if ( scat->elements[0].len != (int) sizeof(token_header) ) {
+	  Alarmp( SPLOG_FATAL, MEMB, "Read_form2: wrong size header %d (should be %d)\n", (int) scat->elements[0].len, (int) sizeof(token_header) );
+	  return;
+	}
+
+	tot_len    = (int) scat->elements[1].len;
+
 	m_info	   = (members_info *)scat->elements[1].buf;
 	num_bytes  += sizeof(members_info);
 
@@ -1979,6 +2039,11 @@
 
 	rg_info= (ring_info    *)&scat->elements[1].buf[num_bytes];
 
+	if ( num_bytes > tot_len ) {
+	        Alarmp( SPLOG_FATAL, MEMB, "Read_form2:%d: malformed packet; num_bytes (%d), tot_len (%d)\n", __LINE__, num_bytes, tot_len );
+		return;
+	}
+
 	if( !Same_endian( form_token->type ) )
 	{
 		Flip_members( m_info );
@@ -1990,9 +2055,19 @@
 	form_token->proc_id = My.id;
 
 	/* validity check */
-	if( m_info->members[m_info->num_members] != My.id ||
-	    m_info->num_pending <= 0 ) return;
+	if( m_info->num_members < 0 || m_info->num_pending <= 0 || (int) m_info->num_members + m_info->num_pending >= MAX_PROCS_RING ||
+	    m_info->members[m_info->num_members] != My.id ) {
+	        Alarmp( SPLOG_FATAL, MEMB, "Read_form2:%d: malformed packet; num_members (%hd), num_pending (%hd), next (0x%08X)\n", __LINE__,
+			m_info->num_members, m_info->num_pending, ( m_info->num_members < MAX_PROCS_RING ? m_info->members[m_info->num_members] : -1 ) );
+		return;
+	}
 
+	/* print */
+	if ( Alarm_get_priority() >= SPLOG_INFO && ( Alarm_get_types() & MEMB ) != 0 ) {
+		Alarmp( SPLOG_INFO, MEMB, "Read_form2: RECEIVED following token:\n" );
+	        Memb_print_form_token( scat );
+	}
+
 	m_info->num_members++;
 	m_info->num_pending--;
 
@@ -2038,22 +2113,25 @@
 	my_holes_procs_ptr = NULL;
 	for( i=0; i < *num_rings; i++ )
 	{
-	    bytes_to_skip = sizeof(ring_info) +
-			    ( rg_info->num_holes + rg_info->num_commit ) * sizeof(int32);
+	    bytes_to_skip = sizeof(ring_info) + ( rg_info->num_holes + rg_info->num_commit ) * sizeof(int32);
 	    if( Memb_is_equal( rg_info->memb_id, Membership_id ) )
 	    {
 		my_rg_info = rg_info;
-		my_holes_procs_ptr = 
-		    (int32 *)&scat->elements[1].buf[num_bytes+sizeof(ring_info)];
+		my_holes_procs_ptr = (int32 *)&scat->elements[1].buf[num_bytes+sizeof(ring_info)];
 	    }
 	    c_ptr = (char *) rg_info;
 	    rg_info = (ring_info *)&c_ptr[bytes_to_skip];
 	    num_bytes   += bytes_to_skip;
+	    
+	    if ( num_bytes > tot_len ) {
+	            Alarmp( SPLOG_FATAL, MEMB, "Read_form2:%d: malformed packet; num_bytes (%d), tot_len (%d)\n", __LINE__, num_bytes, tot_len );
+		    return;
+	    }
 	}
 
         if (my_rg_info == NULL) {
-                Alarm(EXIT, "Read_form2: num_rings = %d, num_bytes = %d, Memb_id = (%d %d)\n",
-                      *num_rings, num_bytes, Membership_id.proc_id, Membership_id.time); 
+	        Alarmp( SPLOG_FATAL, MEMB, "Read_form2: num_rings = %d, num_bytes = %d, Memb_id = (%d %d)\n",
+			*num_rings, num_bytes, Membership_id.proc_id, Membership_id.time); 
         }
 
 	Alarmp( SPLOG_INFO, MEMB, "Read_form2: updating Highest_seq %d -> %d; Aru %d -> %d\n", Highest_seq, my_rg_info->highest_seq, Aru, my_rg_info->aru );
@@ -2064,10 +2142,12 @@
          *       from the old membership with sequence numbers prior to the old Aru.
          */
 	Discard_packets();
+
+	/* NOTE: my_holes_procs_ptr already bounds checked above */
 	
 	for( i=0; i < my_rg_info->num_holes; i++ )
 	{
-		/* create dummy messages */
+	        /* create dummy messages */ 
 		pack_entry = *my_holes_procs_ptr & PACKET_MASK;
 		Alarm( MEMB , "EXTRACT HOLE IS %d\n",*my_holes_procs_ptr);
 		if( Packets[pack_entry].exist != 0 )




More information about the Spread-cvs mailing list