whitelist: Add error handling to socket reads and fix a memory leak
This now adds some state (basically a message buffer), but simplifies error handling as we don't have to handle two potential failure paths and could avoid some potential issues by still calling the blocking read_all(). It also fixes a memory leak when clients disconnect.
This commit is contained in:
@@ -92,38 +92,51 @@ static void list(private_whitelist_control_t *this,
|
|||||||
stream->write_all(stream, &msg, sizeof(msg));
|
stream->write_all(stream, &msg, sizeof(msg));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Information about a client connection.
|
||||||
|
*/
|
||||||
|
typedef struct {
|
||||||
|
private_whitelist_control_t *this;
|
||||||
|
whitelist_msg_t msg;
|
||||||
|
size_t read;
|
||||||
|
} whitelist_conn_t;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Dispatch a received message
|
* Dispatch a received message
|
||||||
*/
|
*/
|
||||||
CALLBACK(on_read, bool,
|
CALLBACK(on_read, bool,
|
||||||
private_whitelist_control_t *this, stream_t *stream)
|
whitelist_conn_t *conn, stream_t *stream)
|
||||||
{
|
{
|
||||||
|
private_whitelist_control_t *this = conn->this;
|
||||||
identification_t *id;
|
identification_t *id;
|
||||||
whitelist_msg_t msg;
|
ssize_t len;
|
||||||
ssize_t n;
|
|
||||||
|
|
||||||
while (TRUE)
|
while (TRUE)
|
||||||
{
|
{
|
||||||
n = stream->read(stream, &msg, sizeof(msg), FALSE);
|
while (conn->read < sizeof(conn->msg))
|
||||||
if (n <= 0)
|
|
||||||
{
|
{
|
||||||
if (errno == EWOULDBLOCK)
|
len = stream->read(stream, (char*)&conn->msg + conn->read,
|
||||||
{
|
sizeof(conn->msg) - conn->read, FALSE);
|
||||||
break;
|
if (len <= 0)
|
||||||
}
|
|
||||||
return FALSE;
|
|
||||||
}
|
|
||||||
if (n < sizeof(msg))
|
|
||||||
{
|
|
||||||
if (!stream->read_all(stream, ((char *)&msg) + n, sizeof(msg) - n))
|
|
||||||
{
|
{
|
||||||
|
if (errno == EWOULDBLOCK)
|
||||||
|
{
|
||||||
|
return TRUE;
|
||||||
|
}
|
||||||
|
if (len != 0)
|
||||||
|
{
|
||||||
|
DBG1(DBG_CFG, "whitelist socket error: %s", strerror(errno));
|
||||||
|
}
|
||||||
|
stream->destroy(stream);
|
||||||
|
free(conn);
|
||||||
return FALSE;
|
return FALSE;
|
||||||
}
|
}
|
||||||
|
conn->read += len;
|
||||||
}
|
}
|
||||||
|
|
||||||
msg.id[sizeof(msg.id) - 1] = 0;
|
conn->msg.id[sizeof(conn->msg.id) - 1] = 0;
|
||||||
id = identification_create_from_string(msg.id);
|
id = identification_create_from_string(conn->msg.id);
|
||||||
switch (ntohl(msg.type))
|
switch (ntohl(conn->msg.type))
|
||||||
{
|
{
|
||||||
case WHITELIST_ADD:
|
case WHITELIST_ADD:
|
||||||
this->listener->add(this->listener, id);
|
this->listener->add(this->listener, id);
|
||||||
@@ -148,6 +161,7 @@ CALLBACK(on_read, bool,
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
id->destroy(id);
|
id->destroy(id);
|
||||||
|
conn->read = 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
return TRUE;
|
return TRUE;
|
||||||
@@ -156,7 +170,12 @@ CALLBACK(on_read, bool,
|
|||||||
CALLBACK(on_accept, bool,
|
CALLBACK(on_accept, bool,
|
||||||
private_whitelist_control_t *this, stream_t *stream)
|
private_whitelist_control_t *this, stream_t *stream)
|
||||||
{
|
{
|
||||||
stream->on_read(stream, on_read, this);
|
whitelist_conn_t *conn;
|
||||||
|
|
||||||
|
INIT(conn,
|
||||||
|
.this = this,
|
||||||
|
);
|
||||||
|
stream->on_read(stream, on_read, conn);
|
||||||
return TRUE;
|
return TRUE;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user