select() call corrupts stack

select() call corrupts stack

[Artiom Morozov]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

Hello,

	Here's a sample program. Try running it and open about 2k of 
connections to port 5222 (you'll need ulimit -n 10000 or like that). It 
will segfault. Simple asm like this
   __asm__(
	"pushl %eax \n\t" 	"movl  0(%ebp), %eax \n\t"
	"cmp   $65535, %eax \n\t"
	"ja isok \n\t"
	"xor  %eax, %eax \n\t"
	"movl  %eax, 0(%eax) \n\t"	 
	"isok: \n\t"
	"popl  %eax \n\t"
   );
after each subroutine call will show you that after select() [ebp] have 
weird value. While this is unlikely to be a security flaw, i think this 
is a bug.

ps: it's okay for 1k of connections or so
pps: kernel 2.4.17 on i686, gcc 3.0.3, glibc 2.2.3.

[-- Attachment #2: main.cpp --]
[-- Type: text/x-c++, Size: 2789 bytes --]

#include <deque>
#include <pthread.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/time.h>
#include <unistd.h>
#include <stdio.h>
#include <errno.h>
#include <string.h>
#include <netinet/in.h>

using namespace std;

typedef void* (*DISP_ENTRY_ROUTINE)(void*);

typedef struct
{
  pthread_mutex_t sockLock;
  int sock;
  DISP_ENTRY_ROUTINE disp;
} ReqInfo;

pthread_mutex_t reqsLock = PTHREAD_MUTEX_INITIALIZER;
deque<ReqInfo*> reqs;

void* disp0(void*);

void add_to_queue(int s)
{
  pthread_mutex_lock(&reqsLock);

  ReqInfo * nInfo = new ReqInfo[1];
  nInfo->sock = s;
  pthread_mutex_init(&nInfo->sockLock, NULL);
  nInfo->disp = disp0;
	  
  reqs.push_back(nInfo);
	  
  pthread_mutex_unlock(&reqsLock);
}

void* disp0(void* param)
{
  ReqInfo *p = (ReqInfo *) param;
  char foo[0x10];

  recv(p->sock, foo, 4, 0);
  
  add_to_queue(p->sock);
  return NULL;
}

void* proc_acc(void* param)
{
  int s = socket(PF_INET, SOCK_STREAM, 0);
  
  if (s == -1)
  {
	fprintf(stderr, "proc_acc: socket() failed, %s\n", strerror(errno));
	return NULL;
  }
	
  struct sockaddr_in saL;
  saL.sin_family = AF_INET;
  saL.sin_port = htons(5222);
  saL.sin_addr.s_addr = htonl(INADDR_ANY);
		
  if (bind(s, (sockaddr*)&saL, sizeof(saL)) == -1)
  {
	fprintf(stderr, "proc_acc: bind() failed, %s\n", strerror(errno));
	return NULL;
  }
  
  if (listen(s, SOMAXCONN) == -1)
  {
	fprintf(stderr, "proc_acc: listen() failed, %s\n", strerror(errno));
	return NULL;
  }

  do
  {
	int len;
	struct sockaddr_in addr;
	int ch = accept(s, (struct sockaddr*) &addr, (socklen_t*) &len);
	if (ch == -1)
	{
	  fprintf(stderr, "proc_acc: accept() failed, %s\n", strerror(errno));
	}
	else
	{
	  add_to_queue(ch);
	}
  } while (1);

  return NULL;
}

int try_select(ReqInfo *p)
{
  fd_set set;
  struct timeval tv;
  int out = 0;
  
  do
  {
	tv.tv_sec = tv.tv_usec = 0;
	FD_ZERO(&set);
	FD_SET(p->sock, &set);
  } while ((out = select(p->sock + 1, &set, NULL, NULL, &tv)) == -1 
			&& errno == EINTR);
  
  return out;
}

void* proc_sel(void* param)
{
  do
  {
	ReqInfo *p = NULL;
	pthread_mutex_lock(&reqsLock);
	if (reqs.size())
	{
	  p = reqs.front();
	  reqs.pop_front();
	}
	pthread_mutex_unlock(&reqsLock);
	
	if (p != NULL)
	{
	  int r = try_select(p);
	  switch (r)
	  {
		case 0: 
		{ 
		  pthread_mutex_lock(&reqsLock); 
		  reqs.push_back(p);
		  pthread_mutex_unlock(&reqsLock); 
		}; break;
		case -1:
		{
		  fprintf(stderr, "select(): %s\n", strerror(errno));
		}; 
		default:
		{
		  p->disp(p);
		  delete[] p;
		}; 
	  }
	}
  } while (1);
}

int main()
{
  pthread_t th_acc, th_sel;
  
  pthread_create(&th_acc, NULL, proc_acc, NULL);
  pthread_create(&th_sel, NULL, proc_sel, NULL);

  // should crash here, so don't care about cleanup

  pthread_join(th_acc, NULL);

  return 0;
}

[-- Attachment #3: Makefile --]
[-- Type: text/x-makefile, Size: 53 bytes --]

all:
	c++ -pthread -g -Wall -o test-acceptor main.cpp

Re: select() call corrupts stack

[Brian Gerst]

Artiom Morozov wrote:
> 
> Hello,
> 
>         Here's a sample program. Try running it and open about 2k of
> connections to port 5222 (you'll need ulimit -n 10000 or like that). It
> will segfault. Simple asm like this
>    __asm__(
>         "pushl %eax \n\t"       "movl  0(%ebp), %eax \n\t"
>         "cmp   $65535, %eax \n\t"
>         "ja isok \n\t"
>         "xor  %eax, %eax \n\t"
>         "movl  %eax, 0(%eax) \n\t"
>         "isok: \n\t"
>         "popl  %eax \n\t"
>    );
> after each subroutine call will show you that after select() [ebp] have
> weird value. While this is unlikely to be a security flaw, i think this
> is a bug.
> 
> ps: it's okay for 1k of connections or so
> pps: kernel 2.4.17 on i686, gcc 3.0.3, glibc 2.2.3.
> 
>   ------------------------------------------------------------------------
> 
>    main.cppName: main.cpp
>            Type: C++ Source file (application/x-unknown-content-type-cppfile)
> 
>    MakefileName: Makefile
>            Type: text/x-makefile


This is not a kernel problem.  You are overflowing the size of the
fd_set structure in userspace, which has room for FD_SETSIZE file
descriptors.  The kernel smashes the user stack because you told it you
had more descriptors that you gave it room for.

--

				Brian Gerst

Re: select() call corrupts stack

[Andreas Schwab]

Artiom Morozov <artiom@phreaker.net> writes:

|> Hello,
|> 
|> 	Here's a sample program. Try running it and open about 2k of
|> 	connections to port 5222 (you'll need ulimit -n 10000 or like
|> 	that). It will segfault. Simple asm like this
|>    __asm__(
|> 	"pushl %eax \n\t" 	"movl  0(%ebp), %eax \n\t"
|> 	"cmp   $65535, %eax \n\t"
|> 	"ja isok \n\t"
|> 	"xor  %eax, %eax \n\t"
|> 	"movl  %eax, 0(%eax) \n\t"	 	"isok: \n\t"
|> 	"popl  %eax \n\t"
|>    );
|> after each subroutine call will show you that after select() [ebp] have
|> weird value. While this is unlikely to be a security flaw, i think this is
|> a bug.
|> 
|> ps: it's okay for 1k of connections or so

/* Number of descriptors that can fit in an `fd_set'.  */
#define __FD_SETSIZE	1024

Use poll(3) instead.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE GmbH, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: select() call corrupts stack

[Jakob Østergaard]

On Wed, Feb 27, 2002 at 09:40:56PM +0200, Artiom Morozov wrote:
> Hello,
> 
> 	Here's a sample program. Try running it and open about 2k of 
> connections to port 5222 (you'll need ulimit -n 10000 or like that). It 
> will segfault. Simple asm like this
>    __asm__(
> 	"pushl %eax \n\t" 	"movl  0(%ebp), %eax \n\t"
> 	"cmp   $65535, %eax \n\t"
> 	"ja isok \n\t"
> 	"xor  %eax, %eax \n\t"
> 	"movl  %eax, 0(%eax) \n\t"	 
> 	"isok: \n\t"
> 	"popl  %eax \n\t"
>    );
> after each subroutine call will show you that after select() [ebp] have 
> weird value. While this is unlikely to be a security flaw, i think this 
> is a bug.
> 
> ps: it's okay for 1k of connections or so
> pps: kernel 2.4.17 on i686, gcc 3.0.3, glibc 2.2.3.

It's most likely not a kernel bug.  *Probably* not a glibc bug either.  Please
understand that *many* applications rely on select() working properly - finding
bugs in these basic system calls is rare (but not unheard of).

I'm going to send a long rant here - I really don't want to insult you, but I
cannot explain what I mean without giving some harsh comments. If you can't
handle that - stop reading now  :)

<rant>

First of all;  your code is ugly.  Sorry, but it is.  You cannot expect dirty
code to work,  and you *especially* cannot expect dirty *threaded* code to
work.

If you want to use C++, you should use it properly.  Like, not using void*
where real types would do.  And not using NULL. And not typecasting anonymous
structs. And not making deques of pointers. And, and, and...    If you cannot
do that,  then you should refrain from using C++, as you will avoid some
expensive mistakes by using a simpler and more primitive language, such as C.

Poor C++ is *a*lot* worse than poor C.  (The door swings both ways though)

May I recommend:  "The C++ Programming Language, 3rd edition", by a fellow
dane, Bjarne Stroustrup - and of course his other most excellent book "The
Design and Evolution of C++".   Read those two, and you will never write code
like the stuff you posted here again   :)

Furthermore, you abuse threads massively. You should read up on non-blocking
communication:  either just the man-pages, or using books like "UNIX Systems
Programming, for SVR5" or others..  

As someone (Alan I think) said:
  Threads are for people who cannot
  program state machines.

I would add:  "or people who need concurrent *computations* on multiple CPUs",
but that's obviously not what your application is trying to do.

</rant>

So my advice:
 *)  Start using C++, or stop using it.  There is no good compromise.
 *)  Read about non-blocking communication.
 *)  Run ElectricFence on code that has memory corruption

Cheers,

-- 
................................................................
:   jakob@unthought.net   : And I see the elder races,         :
:.........................: putrid forms of man                :
:   Jakob Østergaard      : See him rise and claim the earth,  :
:        OZ9ABN           : his downfall is at hand.           :
:.........................:............{Konkhra}...............:

Re: select() call corrupts stack

[Byron Stanoszek]

On Wed, 27 Feb 2002, Artiom Morozov wrote:

> Hello,
> 
> 	Here's a sample program. Try running it and open about 2k of 
> connections to port 5222 (you'll need ulimit -n 10000 or like that). It 
> will segfault. Simple asm like this

> ps: it's okay for 1k of connections or so

printf("%d\n", sizeof(fd_set));

You'll see that it's 128, which is equivalent to 1024 bits, or 1024 possible
open connections that can be held in that variable. I would consider using a
dynamically-resizable memory space like in the code below to do what you want:

    struct timeval timeout;
    static fd_set *input_set, *output_set;
    static int fdset_size;

    extern int top_desc;

    /* dynamically grow file descriptor sets when needed (for >1024 FDs) */
    if(!input_set || (fdset_size-8)*8 <= top_desc) {
      free(input_set);
      free(output_set);
      fdset_size=(top_desc/64+2)*8;
      input_set=malloc(fdset_size);
      output_set=malloc(fdset_size);
    }

    /* clear input/output file descriptor sets */
    memset(input_set, 0, fdset_size);
    memset(output_set, 0, fdset_size);

    [... insert code to set input_set/output_set bits using FD_SET ...]

    select(top_desc+1, input_set, output_set, NULL, &timeout);


 -Byron

-- 
Byron Stanoszek                         Ph: (330) 644-3059
Systems Programmer                      Fax: (330) 644-8110
Commercial Timesharing Inc.             Email: byron@comtime.com