From 65b62d83ee72012d1171f1813b8f989f8805497c Mon Sep 17 00:00:00 2001 From: Linus Nordberg Date: Wed, 15 May 2013 11:57:09 +0200 Subject: [PATCH] Don't crash on reading invalid messages. Also, invoke disconnected callback and close connection in error cases. --- lib/conn.c | 32 +++++++++++++++++++------------- lib/conn.h | 1 - lib/tcp.c | 12 ++++++++---- lib/udp.c | 32 +++++++++++++------------------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/lib/conn.c b/lib/conn.c index f89ac70..499c330 100644 --- a/lib/conn.c +++ b/lib/conn.c @@ -20,19 +20,6 @@ #include "tcp.h" int -conn_close (struct rs_connection **connp) -{ - int r = 0; - assert (connp); - assert (*connp); - if ((*connp)->is_connected) - r = rs_conn_disconnect (*connp); - if (r == RSE_OK) - *connp = NULL; - return r; -} - -int conn_user_dispatch_p (const struct rs_connection *conn) { assert (conn); @@ -145,6 +132,25 @@ rs_conn_disconnect (struct rs_connection *conn) assert (conn); + if (conn->is_connected) + event_on_disconnect (conn); + + if (conn->bev) + { + bufferevent_free (conn->bev); + conn->bev = NULL; + } + if (conn->rev) + { + event_free (conn->rev); + conn->rev = NULL; + } + if (conn->wev) + { + event_free (conn->wev); + conn->wev = NULL; + } + err = evutil_closesocket (conn->fd); conn->fd = -1; return err; diff --git a/lib/conn.h b/lib/conn.h index dfeaf74..66e15e2 100644 --- a/lib/conn.h +++ b/lib/conn.h @@ -1,7 +1,6 @@ /* Copyright 2011,2013 NORDUnet A/S. All rights reserved. See LICENSE for licensing information. */ -int conn_close (struct rs_connection **connp); int conn_user_dispatch_p (const struct rs_connection *conn); int conn_activate_timeout (struct rs_connection *conn); int conn_type_tls (const struct rs_connection *conn); diff --git a/lib/tcp.c b/lib/tcp.c index 8ea6a5e..e2b1a2f 100644 --- a/lib/tcp.c +++ b/lib/tcp.c @@ -38,7 +38,9 @@ _read_header (struct rs_packet *pkt) pkt->rpkt->length = (pkt->hdr[2] << 8) + pkt->hdr[3]; if (pkt->rpkt->length < 20 || pkt->rpkt->length > RS_MAX_PACKET_LEN) { - conn_close (&pkt->conn); + rs_debug (("%s: invalid packet length: %d\n", + __func__, pkt->rpkt->length)); + rs_conn_disconnect (pkt->conn); return rs_err_conn_push (pkt->conn, RSE_INVALID_PKT, "invalid packet length: %d", pkt->rpkt->length); @@ -55,7 +57,8 @@ _read_header (struct rs_packet *pkt) } else /* Error: libevent gave us less than the low watermark. */ { - conn_close (&pkt->conn); + rs_debug (("%s: got: %d octets reading header\n", __func__, n)); + rs_conn_disconnect (pkg->conn); return rs_err_conn_push_fl (pkt->conn, RSE_INTERNAL, __FILE__, __LINE__, "got %d octets reading header", n); } @@ -100,8 +103,9 @@ _read_packet (struct rs_packet *pkt) err = nr_packet_ok (pkt->rpkt); if (err != RSE_OK) { - conn_close (&pkt->conn); - return rs_err_conn_push_fl (pkt->conn, err, __FILE__, __LINE__, + rs_debug (("%s: %d: invalid packet\n", __func__, -err)); + rs_conn_disconnect (pkt->conn); + return rs_err_conn_push_fl (pkt->conn, -err, __FILE__, __LINE__, "invalid packet"); } diff --git a/lib/udp.c b/lib/udp.c index 5eb0645..36af084 100644 --- a/lib/udp.c +++ b/lib/udp.c @@ -65,22 +65,22 @@ static void _evcb (evutil_socket_t fd, short what, void *user_data) { int err; + struct rs_packet *pkt = (struct rs_packet *) user_data; rs_debug (("%s: fd=%d what =", __func__, fd)); - if (what & EV_TIMEOUT) rs_debug ((" TIMEOUT")); + if (what & EV_TIMEOUT) rs_debug ((" TIMEOUT -- shouldn't happen!")); if (what & EV_READ) rs_debug ((" READ")); if (what & EV_WRITE) rs_debug ((" WRITE")); rs_debug (("\n")); + assert (pkt); + assert (pkt->conn); if (what & EV_READ) { /* Read a single UDP packet and stick it in USER_DATA. */ /* TODO: Verify that unsolicited packets are dropped. */ - struct rs_packet *pkt = (struct rs_packet *) user_data; ssize_t r = 0; - assert (pkt); - assert (pkt->conn); assert (pkt->rpkt->data); r = compat_recv (fd, pkt->rpkt->data, RS_MAX_PACKET_LEN, MSG_TRUNC); @@ -92,7 +92,7 @@ _evcb (evutil_socket_t fd, short what, void *user_data) /* FIXME: Really shouldn't happen since we've been told that fd is readable! */ rs_debug (("%s: EAGAIN reading UDP packet -- wot?")); - return; + goto err_out; } /* Hard error. */ @@ -100,23 +100,22 @@ _evcb (evutil_socket_t fd, short what, void *user_data) "%d: recv: %d (%s)", fd, sockerr, evutil_socket_error_to_string (sockerr)); event_del (pkt->conn->tev); - return; + goto err_out; } event_del (pkt->conn->tev); if (r < 20 || r > RS_MAX_PACKET_LEN) /* Short or long packet. */ { rs_err_conn_push (pkt->conn, RSE_INVALID_PKT, - "invalid packet length: %d", - pkt->rpkt->length); - return; + "invalid packet length: %d", r); + goto err_out; } pkt->rpkt->length = (pkt->rpkt->data[2] << 8) + pkt->rpkt->data[3]; err = nr_packet_ok (pkt->rpkt); if (err) { - rs_err_conn_push_fl (pkt->conn, err, __FILE__, __LINE__, + rs_err_conn_push_fl (pkt->conn, -err, __FILE__, __LINE__, "invalid packet"); - return; + goto err_out; } /* Hand over message to user. This changes ownership of pkt. Don't touch it afterwards -- it might have been freed. */ @@ -125,10 +124,6 @@ _evcb (evutil_socket_t fd, short what, void *user_data) } else if (what & EV_WRITE) { - struct rs_packet *pkt = (struct rs_packet *) user_data; - assert (pkt); - assert (pkt->conn); - if (!pkt->conn->is_connected) event_on_connect (pkt->conn, pkt); @@ -137,11 +132,10 @@ _evcb (evutil_socket_t fd, short what, void *user_data) if (pkt->conn->callbacks.sent_cb) pkt->conn->callbacks.sent_cb (pkt->conn->user_data); } + return; -#if defined (DEBUG) - if (what & EV_TIMEOUT) - rs_debug (("%s: timeout on UDP event, shouldn't happen\n", __func__)); -#endif + err_out: + rs_conn_disconnect (pkt->conn); } int -- 2.1.4