1 files changed, 39 insertions, 18 deletions

diff --git a/openbsc/src/libmsc/smpp_smsc.c b/openbsc/src/libmsc/smpp_smsc.c
index 64ed2005f..048c1b802 100644
--- a/openbsc/src/libmsc/smpp_smsc.c
+++ b/openbsc/src/libmsc/smpp_smsc.c
@@ -23,6 +23,7 @@
 #include <string.h>
 #include <stdint.h>
 #include <errno.h>
+#include <limits.h>
 
 #include <sys/socket.h>
 #include <netinet/in.h>
@@ -762,6 +763,23 @@ static int smpp_pdu_rx(struct osmo_esme *esme, struct msgb *msg __uses)
 	return rc;
 }
 
+/* This macro should be called after a call to read() in the read_cb of an
+ * osmo_fd to properly check for errors.
+ * rc is the return value of read, err_label is the label to jump to in case of
+ * an error. The code there should handle closing the connection.
+ * FIXME: This code should go in libosmocore utils.h so it can be used by other
+ * projects as well.
+ * */
+#define OSMO_FD_CHECK_READ(rc, err_label) \
+	if (rc < 0) { \
+		/* EINTR is a non-fatal error, just try again */ \
+		if (errno == EINTR) \
+			return 0; \
+		goto err_label; \
+	} else if (rc == 0) { \
+		goto err_label; \
+	}
+
 /* !\brief call-back when per-ESME TCP socket has some data to be read */
 static int esme_link_read_cb(struct osmo_fd *ofd)
 {
@@ -770,22 +788,27 @@ static int esme_link_read_cb(struct osmo_fd *ofd)
 	uint8_t *lenptr = (uint8_t *) &len;
 	uint8_t *cur;
 	struct msgb *msg;
-	int rdlen;
-	int rc;
+	ssize_t rdlen, rc;
 
 	switch (esme->read_state) {
 	case READ_ST_IN_LEN:
 		rdlen = sizeof(uint32_t) - esme->read_idx;
 		rc = read(ofd->fd, lenptr + esme->read_idx, rdlen);
-		if (rc < 0) {
-			LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d\n",
-			     esme->system_id, rc);
-		} else if (rc == 0) {
-			goto dead_socket;
-		} else
-			esme->read_idx += rc;
+		if (rc < 0)
+			LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %zd (%s)\n",
+					esme->system_id, rc, strerror(errno));
+		OSMO_FD_CHECK_READ(rc, dead_socket);
+
+		esme->read_idx += rc;
+
 		if (esme->read_idx >= sizeof(uint32_t)) {
 			esme->read_len = ntohl(len);
+			if (esme->read_len < 8 || esme->read_len > UINT16_MAX) {
+				LOGP(DSMPP, LOGL_ERROR, "[%s] length invalid %u\n",
+						esme->system_id, esme->read_len);
+				goto dead_socket;
+			}
+
 			msg = msgb_alloc(esme->read_len, "SMPP Rx");
 			if (!msg)
 				return -ENOMEM;
@@ -800,15 +823,13 @@ static int esme_link_read_cb(struct osmo_fd *ofd)
 		msg = esme->read_msg;
 		rdlen = esme->read_len - esme->read_idx;
 		rc = read(ofd->fd, msg->tail, OSMO_MIN(rdlen, msgb_tailroom(msg)));
-		if (rc < 0) {
-			LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %d\n",
-				esme->system_id, rc);
-		} else if (rc == 0) {
-			goto dead_socket;
-		} else {
-			esme->read_idx += rc;
-			msgb_put(msg, rc);
-		}
+		if (rc < 0)
+			LOGP(DSMPP, LOGL_ERROR, "[%s] read returned %zd (%s)\n",
+					esme->system_id, rc, strerror(errno));
+		OSMO_FD_CHECK_READ(rc, dead_socket);
+
+		esme->read_idx += rc;
+		msgb_put(msg, rc);
 
 		if (esme->read_idx >= esme->read_len) {
 			rc = smpp_pdu_rx(esme, esme->read_msg);