tcp server

0x080491b3 in fchatter (connectSocket=0x8) at tpchatd.cpp:31

Der Pointer connectSocket mit der Adresse 0x8 sieht nicht sehr koscher aus!

Wenn man dan schaut wo und wie die Funktion aufgerufen wird, stellt man folgendes fest:

pthread_create(&chatter, NULL,&fchatter,(int connectSocket);

Du castest einen Integer in einen Pointer auf einen Integer.

Jetzt ein kleiner Blick auf deine Threadfunction

void *fchatter(int *connectSocket)

Jetzt muss dir bewusst sein, was connectSocket in Wirklichkeit ist. Ein Pointer auf ein Integer? Nein! Ein Integer gecasted als Pointer auf einen Integer! Also dereferenzieren ist nicht!

Die libpthread hat nicht umsonst den Parameter als void-Pointer definiert.

mfg Unixer

P.S.: Eigne dir die Basics über Pointer an

entelechie

alle threads benutzen das selbe client socket...
sendet thread a und es connected ein neuer client (->thread b wird erzeugt)
wird der socket von thread a ueberschrieben. boese.

int clientSocket;

clientSocket = accept(...);
...
pthread_create(&chatter, NULL,&fchatter,(int *)clientSocket );
/*
 -> clientSocket liegt auf dem stack, jeder thread besitzt einen zeigen
    auf die selbe int-var
*/

-> moegliche loesung: jedem thread ein mit malloc/new erzeugtes int uebergeben
(der thread raeumts am ende auf).

alle threads benutzen das selbe client socket...
sendet thread a und es connected ein neuer client (->thread b wird erzeugt)
wird der socket von thread a ueberschrieben. boese.

Falsch!

Es heisst:

pthread_create(&chatter, NULL,&fchatter,(int *)clientSocket );

und nicht

pthread_create(&chatter, NULL,&fchatter,(int *) &clientSocket );

Man achte auf das kleine aber feine '&'

mfg unixer

-> moegliche loesung: jedem thread ein mit malloc/new erzeugtes int uebergeben(der thread raeumts am ende auf).

Ansich ist es kein Problem den Descriptor als Param (also der void-Pointer-Parameter) zu übergeben. Man muss beachten, dass man ihn dan auch wirklich als Descriptor verwendet und nicht als Pointer.

unixer

entelechie

unixer schrieb:

alle threads benutzen das selbe client socket...
sendet thread a und es connected ein neuer client (->thread b wird erzeugt)
wird der socket von thread a ueberschrieben. boese.

Falsch!

Es heisst:
pthread_create(&chatter, NULL,&fchatter,(int *)clientSocket );
und nicht
pthread_create(&chatter, NULL,&fchatter,(int *) &clientSocket );
Man achte auf das kleine aber feine '&'

mfg unixer

super. wenn du glueck hast funktioniert das tatsaechlich.
ansi c garantiert das nicht. das funktioniert nur, solange
ein int kleiner oder gleich der groesse eines zeigers ist;
was zwar oft aber nicht zwingend der fall ist.

machs lieber indem fuer fuer jeden thread ein int allokierst
da bist du auf der sicheren seite.

super. wenn du glueck hast funktioniert das tatsaechlich.
ansi c garantiert das nicht. das funktioniert nur, solange
ein int kleiner oder gleich der groesse eines zeigers ist;
was zwar oft aber nicht zwingend der fall ist.

Das ist wahr! Aber ich kenn keine Architektur, auf der die libpthread läuft, bei der das der Fall ist. IMHO muss man nicht solch einen Mehraufwand betreiben um diesen _sehr_ theoretischen Fall abzudecken.

mfg unixer

debian inside

unixer schrieb:

super. wenn du glueck hast funktioniert das tatsaechlich.
ansi c garantiert das nicht. das funktioniert nur, solange
ein int kleiner oder gleich der groesse eines zeigers ist;
was zwar oft aber nicht zwingend der fall ist.

Das ist wahr! Aber ich kenn keine Architektur, auf der die libpthread läuft, bei der das der Fall ist. IMHO muss man nicht solch einen Mehraufwand betreiben um diesen _sehr_ theoretischen Fall abzudecken.

mfg unixer

danke

fazit:
es darf nicht immer die selbe threadid, der selbe socket verwendet werden und ich seh mir pointer an

nman

Igitt, ich weiß schon, warum ich kein C mag.

debian inside

so das läuft jetzt
nicht unbedingt schön programmiert aber is ja schon spät und war ein langer tag

#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <iostream>
#include <string>
#include <ctype.h>
#include <pthread.h>
#include <sstream>

using namespace std;

#define PORT 1550
#define MAX_MSG 100
#define LINE_ARRAY_SIZE (MAX_MSG+1)
#define MAX_THREADS 5

int threads = 0;
int thread = 0;

int connectSocket[MAX_THREADS];

int threadrotate();
void *fchatter(int* thisthread)
{
    int temp = (int) thisthread;
    char line[LINE_ARRAY_SIZE];
    memset(line, 0x0, LINE_ARRAY_SIZE);
    while (recv(connectSocket[temp], line, MAX_MSG, 0) > 0) {
      cout << "Thread" << temp << ": " << line << "\n";

      int i;
      for (i = 0; line[i] != '\0'; i++)
        line[i] = toupper(line[i]);

      //if (send(connectSocket[temp], line, strlen(line) + 1, 0) < 0)
      if (send(connectSocket[temp], line, strlen(line), 0) < 0)
        cerr << "Error: cannot send modified data"; 
      memset(line, 0x0, LINE_ARRAY_SIZE);
    }
    cout << "Thread" << temp << " end" << endl;
    connectSocket[temp] = 0;
    threads--;
}

int main()
{
  int listenSocket, i;
  unsigned short int listenPort;
  socklen_t clientAddressLength;
  struct sockaddr_in clientAddress, serverAddress;
  char line[LINE_ARRAY_SIZE];

  listenPort = PORT;

  listenSocket = socket(AF_INET, SOCK_STREAM, 0);
  if (listenSocket < 0) {
    cerr << "cannot create listen socket";
    exit(1);
  }

  serverAddress.sin_family = AF_INET;
  serverAddress.sin_addr.s_addr = htonl(INADDR_ANY);
  serverAddress.sin_port = htons(listenPort);

  if (bind(listenSocket, (struct sockaddr *) &serverAddress, sizeof(serverAddress)) < 0) {
    cerr << "cannot bind socket";
    exit(1);
  }

  listen(listenSocket, 5);

 pthread_t chatter[MAX_THREADS];
  while (1) {
	while (threads<=MAX_THREADS)
	{
	    cout << "Waiting for TCP connection on port " << listenPort << " ...\n";

	    cout << threads << " Threads running" << endl;
	    clientAddressLength = sizeof(clientAddress);
	    thread = threadrotate();
	    connectSocket[thread] = accept(listenSocket, (struct sockaddr *) &clientAddress, &clientAddressLength);
	    if (connectSocket[thread] < 0) {
	      cerr << "cannot accept connection ";
	      exit(1);
	    }

	    cout << "  connected to " << inet_ntoa(clientAddress.sin_addr);

	    cout << ":" << ntohs(clientAddress.sin_port) << "\n";
	    pthread_create(&chatter[thread], NULL,&fchatter,(int *) thread);
	    threads++;
	}
  }
}

int threadrotate()
{
    int f = 0;
    while(connectSocket[f]!=0) f++;
    return f;
}

debian inside

wäre es möglich das jemand mit einer der beiden versionen ne ddos attacke ausführt?

wäre es möglich das jemand mit einer der beiden versionen ne ddos attacke ausführt?

DDos Attacken sind prinzipiell immer möglich, unabhängig von der verwendeten Serversoftware..

Igitt, ich weiß schon, warum ich kein C mag.

Findest wohl static_cast<void *>(var) schöner als (void var

mfg unixer

entelechie

debian inside schrieb:

so das läuft jetzt
void *fchatter(int* thisthread)
{
int temp = (int) thisthread;
...
}
...
cout << ":" << ntohs(clientAddress.sin_port) << "\n";
pthread_create(&chatter[thread], NULL,&fchatter,(int thread);
threads++;
}
}
}
[/cpp]

jetzt haste ne mischloesung

wichtig ist, dass der hauptthread (in dem das accept ist)
den socket-fd kopiert.
theoretisch kann es passieren, dass der hauptthread einen neuen
socket annimmt bevor der worker-thread den alten socket-fd kopiert
hat.

du wirst um ein dynamisches allokieren eines neuen socket-fd nicht
drum rum kommen.

wenn du das programm nur fuer private zwecke hast, nimm die loesung
von unixer; d.h. lass das "(int thread" stehen und gud is.

wenn du das programm nur fuer private zwecke hast, nimm die loesung
von unixer; d.h. lass das "(int thread" stehen und gud is.

Das war nicht meine Lösung! Meine Lösung bezog sich auf den Socketdeskriptor, der als Pointer gecasted übergeben werden soll. Das ganze global und ohne Zugriffsschutz zu machen ist natürlich Käse!

mfg unixer

P.S.: IMHO sind Threads sind für so ein Server eh ein wenig zu heftig. Verwende doch eine Kombination aus nonblocking I/O und select(). Das ist sowieso einfacher und weniger Fehleranfällig.

nman

unixer schrieb:

Findest wohl static_cast<void *>(var) schöner als (void var

Klar, weil ich ja ständig rohes void*-Zeugs herumschiebe in C++.

<klugscheiß>
Aber auch in C++ kann gibt es rohe Systemcalls, die nun mal alle in C sind
</klugscheiß>

mfg

nman

unixer schrieb:

<klugscheiß>
Aber auch in C++ kann gibt es rohe Systemcalls, die nun mal alle in C sind
</klugscheiß>

Hab nichts gegenteiliges behauptet.
Nur so richtig viel davon halten muss ich ja trotzdem nicht.