diff mbox series

[OpenWrt-Devel,ubus,14/16] ubusd_monitor: fix possible null pointer dereference

Message ID 20191219221125.22646-15-ynezz@true.cz
State Accepted
Delegated to: Petr Štetiar
Headers show
Series GitLab CI, tests, fuzzing, fixes and improvements | expand

Commit Message

Petr Štetiar Dec. 19, 2019, 10:11 p.m. UTC
This dereference could possibly happen if the calloc call fails as the
return value is unchecked. While at it refactor the code little bit to
make it easier to follow, use safe list iterator and provide return
value for ubusd_monitor_connect.

Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 ubusd_monitor.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)
diff mbox series

Patch

diff --git a/ubusd_monitor.c b/ubusd_monitor.c
index fcbc6a4b10c0..501e06d0716c 100644
--- a/ubusd_monitor.c
+++ b/ubusd_monitor.c
@@ -29,7 +29,7 @@  ubusd_monitor_free(struct ubus_monitor *m)
 	free(m);
 }
 
-static void
+static bool
 ubusd_monitor_connect(struct ubus_client *cl, struct ubus_msg_buf *ub)
 {
 	struct ubus_monitor *m;
@@ -37,22 +37,40 @@  ubusd_monitor_connect(struct ubus_client *cl, struct ubus_msg_buf *ub)
 	ubusd_monitor_disconnect(cl);
 
 	m = calloc(1, sizeof(*m));
+	if (!m)
+		return false;
+
 	m->cl = cl;
 	list_add(&m->list, &monitors);
+
+	return true;
 }
 
-void
-ubusd_monitor_disconnect(struct ubus_client *cl)
+static struct ubus_monitor*
+ubusd_monitor_find(struct ubus_client *cl)
 {
-	struct ubus_monitor *m;
+	struct ubus_monitor *m, *tmp;
 
-	list_for_each_entry(m, &monitors, list) {
+	list_for_each_entry_safe(m, tmp, &monitors, list) {
 		if (m->cl != cl)
 			continue;
 
-		ubusd_monitor_free(m);
-		return;
+		return m;
 	}
+
+	return NULL;
+}
+
+void
+ubusd_monitor_disconnect(struct ubus_client *cl)
+{
+	struct ubus_monitor *m;
+
+	m = ubusd_monitor_find(cl);
+	if (!m)
+		return;
+
+	ubusd_monitor_free(m);
 }
 
 void
@@ -92,13 +110,15 @@  ubusd_monitor_recv(struct ubus_client *cl, struct ubus_msg_buf *ub,
 		return UBUS_STATUS_PERMISSION_DENIED;
 
 	if (!strcmp(method, "add")) {
-		ubusd_monitor_connect(cl, ub);
-		return 0;
+		if (!ubusd_monitor_connect(cl, ub))
+			return UBUS_STATUS_UNKNOWN_ERROR;
+
+		return UBUS_STATUS_OK;
 	}
 
 	if (!strcmp(method, "remove")) {
 		ubusd_monitor_disconnect(cl);
-		return 0;
+		return UBUS_STATUS_OK;
 	}
 
 	return UBUS_STATUS_METHOD_NOT_FOUND;