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:
Tobias Brunner
2025-07-18 12:07:45 +02:00
parent 412231eecd
commit 85ebf6abd4
@@ -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;
} }