Change in libosmo-abis[master]: ipaccess: Avoid using released line and bfd in ipaccess_fd_cb

[Pau Espin Pedrol]

Pau Espin Pedrol has uploaded this change for review. ( https://gerrit.osmocom.org/9263


Change subject: ipaccess: Avoid using released line and bfd in ipaccess_fd_cb
......................................................................

ipaccess: Avoid using released line and bfd in ipaccess_fd_cb

Related: OS#3282

Change-Id: I52faa9e6717137a7dab9c4e006eaa50b7367fc3e
---
M src/input/ipaccess.c
1 file changed, 12 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/63/9263/1

diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index bbdc5af..e46b0ab 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -84,6 +84,8 @@
 	return ret;
 }
 
+/* Returns -1 on error, and 0 or 1 on success. If -1 or 1 is returned, line has
+ * been release and should not be used anymore by the caller. */
 static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg,
 			   struct osmo_fd *bfd)
 {
@@ -123,13 +125,11 @@
 		if (ret < 0) {
 			LOGP(DLINP, LOGL_ERROR, "IPA response message "
 				"with malformed TLVs\n");
-			ret = -EINVAL;
 			goto err;
 		}
 		if (!TLVP_PRESENT(&tlvp, IPAC_IDTAG_UNIT)) {
 			LOGP(DLINP, LOGL_ERROR, "IPA response message "
 				"without unit ID\n");
-			ret = -EINVAL;
 			goto err;
 
 		}
@@ -137,7 +137,6 @@
 		if (len < 1) {
 			LOGP(DLINP, LOGL_ERROR, "IPA response message "
 				"with too small unit ID\n");
-			ret = -EINVAL;
 			goto err;
 		}
 		unitid = (char *) TLVP_VAL(&tlvp, IPAC_IDTAG_UNIT);
@@ -147,7 +146,6 @@
 		if (!line->ops->sign_link_up) {
 			LOGP(DLINP, LOGL_ERROR,
 			     "Unable to set signal link, closing socket.\n");
-			ret = -EINVAL;
 			goto err;
 		}
 		/* the BSC creates the new sign links at this stage. */
@@ -159,7 +157,6 @@
 				LOGP(DLINP, LOGL_ERROR,
 					"Unable to set signal link, "
 					"closing socket.\n");
-				ret = -EINVAL;
 				goto err;
 			}
 		} else if (bfd->priv_nr == E1INP_SIGN_RSL) {
@@ -174,7 +171,6 @@
 				LOGP(DLINP, LOGL_ERROR,
 					"Unable to set signal link, "
 					"closing socket.\n");
-				ret = -EINVAL;
 				goto err;
 			}
 			/* this is a bugtrap, the BSC should be using the
@@ -184,7 +180,7 @@
 					"Fix your BSC, you should use the "
 					"E1 line used by the OML link for "
 					"your RSL link.\n");
-				return 0;
+				goto err;
 			}
 			/* Finally, we know which OML link is associated with
 			 * this RSL link, attach it to this socket. */
@@ -210,11 +206,11 @@
 			}
 			/* now we can release the dummy RSL line. */
 			e1inp_line_put(line);
+			return 1;
 		}
 		break;
 	default:
 		LOGP(DLINP, LOGL_ERROR, "Unknown IPA message type\n");
-		ret = -EINVAL;
 		goto err;
 	}
 	return 0;
@@ -223,9 +219,10 @@
 	close(bfd->fd);
 	bfd->fd = -1;
 	e1inp_line_put(line);
-	return ret;
+	return -1;
 }
 
+/* Returns -EBADF if bfd cannot be used by the caller anymore after return. */
 static int handle_ts1_read(struct osmo_fd *bfd)
 {
 	struct e1inp_line *line = bfd->data;
@@ -251,23 +248,21 @@
 
 	hh = (struct ipaccess_head *) msg->data;
 	if (hh->proto == IPAC_PROTO_IPACCESS) {
-		ipaccess_rcvmsg(line, msg, bfd);
+		ret = ipaccess_rcvmsg(line, msg, bfd);
+		/* BIG FAT WARNING: bfd might no longer exist here (ret != 0),
+		 * since ipaccess_rcvmsg() might have free'd it !!! */
 		msgb_free(msg);
-		return 0;
+		return ret != 0 ? -EBADF : 0;
 	} else if (e1i_ts->type == E1INP_TS_TYPE_NONE) {
 		/* this sign link is not know yet.. complain. */
 		LOGP(DLINP, LOGL_ERROR, "Timeslot is not configured.\n");
-		ret = -EINVAL;
 		goto err_msg;
 	}
-	/* BIG FAT WARNING: bfd might no longer exist here, since ipaccess_rcvmsg()
-	 * might have free'd it !!! */
 
 	link = e1inp_lookup_sign_link(e1i_ts, hh->proto, 0);
 	if (!link) {
 		LOGP(DLINP, LOGL_ERROR, "no matching signalling link for "
 			"hh->proto=0x%02x\n", hh->proto);
-		ret = -EINVAL;
 		goto err_msg;
 	}
 	msg->dst = link;
@@ -276,7 +271,6 @@
 	if (!e1i_ts->line->ops->sign_link) {
 		LOGP(DLINP, LOGL_ERROR, "Fix your application, "
 			"no action set for signalling messages.\n");
-		ret = -EINVAL;
 		goto err_msg;
 	}
 	rc = e1i_ts->line->ops->sign_link(msg);
@@ -294,7 +288,7 @@
 	msgb_free(msg);
 err:
 	ipaccess_drop(bfd, line);
-	return ret;
+	return -EBADF;
 }
 
 static int ts_want_write(struct e1inp_ts *e1i_ts)
@@ -395,7 +389,7 @@
 
 	if (what & BSC_FD_READ)
 		rc = handle_ts1_read(bfd);
-	if (what & BSC_FD_WRITE)
+	if (rc!=-EBADF && (what & BSC_FD_WRITE))
 		rc = handle_ts1_write(bfd);
 
 	return rc;

-- 
To view, visit https://gerrit.osmocom.org/9263
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I52faa9e6717137a7dab9c4e006eaa50b7367fc3e
Gerrit-Change-Number: 9263
Gerrit-PatchSet: 1
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180523/279407a0/attachment.htm>