I am working on a program that uses lwIP library and turns STM32F769I-DISCO into a TCP client that connects to a TCP server (this role is being fulfilled by Hercules Setup Utility as for now). I made a simple function tcpClientCloseConnection() that frees a custom structure tcpClientStruct and TCP protocol control block (tcp_pcb structure from lwIP) when the TCP connection is supposed to be closed. It frees tcp_pcb inside of tcpClientStruct first (tcp_close() function frees it), then tcpClientStruct.
The problem is, when tcpClientStruct is freed, tcpClientStruct->pcb is no longer NULL and its address changes from 0x0. Because of that, when tcpClientCloseConnection() is invoked again, it returns false when it checks if tcp_pcb is NULL and tries to close it, causing the program to end up in HardFault_Handler() function.
I could check if my custom structure is NULL and not try to free tcp_pcb inside of it if it isn't, but shouldn't tcp_pcb be NULL anyway? I am confused by the way my program behaves and I don't know what could be the reason behind it. I'll include initialising functions in case the problem's cause could be inside of them.
Thank you for help and suggestions in advance!
//main.c
//...
#define SERVER_PORT (33333)
#define SERVER_IP ("192.168.0.103")
ip_addr_t serverIP;
tcpClientStruct *tcpClientInfo=NULL;
//...
int main(void) {
//...
ipaddr_aton(SERVER_IP, &serverIP);
//...
while(1) {
//...
uint8_t conn=verifyConnectionProblems(&tcpClientInfo, SERVER_PORT, serverIP);
//...
}
}
//tcpClient.h
typedef struct {
uint8_t state; //current connection state
uint8_t retries;
struct tcp_pcb *pcb; //pointer to the current tcp_pcb
struct pbuf *packetBuffer; //pointer to received/to be transmitted data
} tcpClientStruct;
//...
//tcpClient.c
int8_t verifyConnectionProblems(tcpClientStruct **tcpClientInfo, uint16_t serverPort, ip_addr_t serverIP) {
//...
tcpClientCloseConnection(&((*tcpClientInfo)->pcb), tcpClientInfo);
//...
}
void tcpClientCloseConnection(struct tcp_pcb **tcpPCB, tcpClientStruct **clientStr) {
if (tcpPCB!=NULL && *tcpPCB!=NULL) {
tcp_close(*tcpPCB);
*tcpPCB=NULL;
}
if (clientStr!=NULL && *clientStr!=NULL) { //free clientStruct
if ((*clientStr)->packetBuffer!=NULL) {
mem_free((*clientStr)->packetBuffer);
(*clientStr)->packetBuffer=NULL;
}
asm("NOP"); //tcpClientInfo addr=0x20005d24 <ram_heap+8>, tcpClientInfo->pcb addr=0x0
mem_free(*clientStr);
asm("NOP"); //tcpClientInfo addr=0x20005d24 <ram_heap+8>, tcpClientInfo->pcb addr=0x0
*clientStr=NULL;
asm("NOP"); //tcpClientInfo addr=0, tcpClientInfo->pcb 0xf73d6e6a
}
}
//initialising functions
uint8_t serverDisconnected=0;
int8_t tcpPCBReinit(struct tcp_pcb **tcpPCB) {
if (tcpPCB!=NULL && *tcpPCB!=NULL) {
mem_free(*tcpPCB);
*tcpPCB=NULL;
}
*tcpPCB=tcp_new(); //create new TCP protocol control block
if (*tcpPCB==NULL) {
return ERR_MEM;
}
return ERR_OK;
}
int8_t tcpClientStructReinit(tcpClientStruct **clientStr) {
if (*clientStr!=NULL) {
if ((*clientStr)->packetBuffer!=NULL) {
mem_free((*clientStr)->packetBuffer);
(*clientStr)->packetBuffer=NULL;
}
mem_free(*clientStr);
*clientStr=NULL;
}
*clientStr=(tcpClientStruct *)mem_malloc(sizeof(tcpClientStruct));
if (*clientStr==NULL) {
return ERR_MEM;
} else {
tcpPCBReinit(&(*clientStr)->pcb);
if ((*clientStr)->pcb==NULL) {
mem_free(*clientStr);
*clientStr = NULL;
return ERR_MEM;
}
(*clientStr)->state=CLIENT_STATUS_NONE;
(*clientStr)->retries=0;
(*clientStr)->packetBuffer=NULL;
serverDisconnected=0;
}
return ERR_OK;
}
int8_t tcpClientInit(const ip_addr_t *ipAddr, uint16_t port, tcpClientStruct **clientStr) {
int8_t errorClientStrReinit=tcpClientStructReinit(clientStr);
if (errorClientStrReinit!=ERR_OK) {
return errorClientStrReinit;
}
tcp_arg((*clientStr)->pcb, (*clientStr)); //specifies the argument that should be passed to all other callback functions
tcp_err((*clientStr)->pcb, tcpClientErrorCallback);
//connect to the server
int8_t connected=tcp_connect((*clientStr)->pcb, ipAddr, port, tcpClientConnectedCallback); //connect to host and invoke callback once connected successfully
if (connected!=ERR_OK) {
serverDisconnected=1;
}
else {
serverDisconnected=0;
}
return connected;
}
//rest of functions...
I think the problem is that verifyConnectionProblems can sometimes call tcpClientCloseConnection with an invalid but non-null pointer value in the first argument (for the tcpPCB parameter).
verifyConnectionProblems is being called in a loop:
while(1) {
//...
uint8_t conn=verifyConnectionProblems(&tcpClientInfo, SERVER_PORT, serverIP);
//...
}
verifyConnectionProblems can call tcpClientCloseConnection:
tcpClientCloseConnection(&((*tcpClientInfo)->pcb), tcpClientInfo);
OP has confirmed that verifyConnectionProblems does not check the value of *tcpClientInfo before the above call to tcpClientCloseConnection.
Assuming the connection has not been closed yet, the call to tcpClientCloseConnection will set *tcpClientInfo to NULL via parameter clientStr (the second parameter of tcpClientCloseConnection):
mem_free(*clientStr);
*clientStr=NULL;
The next time verifyConnectionProblems is called, it may call tcpClientCloseConnection client again:
tcpClientCloseConnection(&((*tcpClientInfo)->pcb), tcpClientInfo);
If *tcpClientInfo is still NULL, the first argument of the call to tcpClientCloseConnection given by the expression &((*tcpClientInfo)->pcb) is an invalid pointer value since it is based on a null pointer value. Since the pcb member is at a non-zero offset from the start of the type tcpClientStruct, typically the pointer value will be invalid but non-null. The first argument is passed to the first parameter tcpPCB.
tcpClientCloseConnection does some validity checking of the parameter tcpPCB:
if (tcpPCB!=NULL && *tcpPCB!=NULL) {
If tcpPCB has an invalid but non-null value, the first part of the test (tcpPCB!=NULL) will be true, but the second part of the test (*tcpPCB!=NULL) will dereference an invalid pointer, resulting in undefined behavior.
We do not know what other tests verifyConnectionProblems does before deciding to call tcpClientCloseConnection, but it ought to at least avoid calling it with invalid, non-null pointers. In particular, it ought to check that *tcpClientInfo is a non-null pointer value.