[PATCH] osmo-bts[master]: Check readv() return value to prevent crash

[Max]

Hello daniel, Neels Hofmeyr, Jenkins Builder,

I'd like you to reexamine a change.  Please visit

    https://gerrit.osmocom.org/3878

to look at the new patch set (#5).

Check readv() return value to prevent crash

Previously result of readv() was used unconditionally so when it failed
and returned negative value it was treated like very large positive
which lead to memory corruption. Fix this and add proper error log.

Change-Id: I956c8d551f45c9dd43b5e9de11dfe20dd8783647
Related: SYS#3865
---
M src/osmo-bts-litecell15/l1_transp_hw.c
M src/osmo-bts-sysmo/l1_transp_hw.c
2 files changed, 16 insertions(+), 4 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/78/3878/5

diff --git a/src/osmo-bts-litecell15/l1_transp_hw.c b/src/osmo-bts-litecell15/l1_transp_hw.c
index 6381864..b526108 100644
--- a/src/osmo-bts-litecell15/l1_transp_hw.c
+++ b/src/osmo-bts-litecell15/l1_transp_hw.c
@@ -204,9 +204,15 @@
 		iov[i].iov_len = msgb_tailroom(msg[i]);
 	}
 
-
 	rc = readv(ofd->fd, iov, ARRAY_SIZE(iov));
-	count = rc / prim_size;
+	if (rc < 0) {
+		LOGP(DL1C, LOGL_ERROR, "failed to read from fd: %s\n", strerror(errno));
+		/* N. B: we do not abort to let the cycle below cleanup allocated memory properly,
+		   the return value is ignored by the caller anyway.
+		   TODO: use libexplain's explain_readv() to provide detailed error description */
+		count = 0;
+	} else
+		count = rc / prim_size;
 
 	for (i = 0; i < count; ++i) {
 		msgb_put(msg[i], prim_size);
diff --git a/src/osmo-bts-sysmo/l1_transp_hw.c b/src/osmo-bts-sysmo/l1_transp_hw.c
index da8ac3f..9c0a514 100644
--- a/src/osmo-bts-sysmo/l1_transp_hw.c
+++ b/src/osmo-bts-sysmo/l1_transp_hw.c
@@ -215,9 +215,15 @@
 		iov[i].iov_len = msgb_tailroom(msg[i]);
 	}
 
-
 	rc = readv(ofd->fd, iov, ARRAY_SIZE(iov));
-	count = rc / prim_size;
+	if (rc < 0) {
+		LOGP(DL1C, LOGL_ERROR, "failed to read from fd: %s\n", strerror(errno));
+		/* N. B: we do not abort to let the cycle below cleanup allocated memory properly,
+		   the return value is ignored by the caller anyway.
+		   TODO: use libexplain's explain_readv() to provide detailed error description */
+		count = 0;
+	} else
+		count = rc / prim_size;
 
 	for (i = 0; i < count; ++i) {
 		msgb_put(msg[i], prim_size);

-- 
To view, visit https://gerrit.osmocom.org/3878
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I956c8d551f45c9dd43b5e9de11dfe20dd8783647
Gerrit-PatchSet: 5
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>