[OpenWrt-Devel] [PATCH ubus 12/16] workaround possibly false positive uses of memory after it is freed

[Petr Štetiar]

scan-build from clang-9 has reported following:

 libubox/list.h:83:22: warning: Use of memory after it is freed
        entry->next->prev = entry->prev;
                            ^~~~~~~~~~~

 ubusd_event.c:42:3: warning: Use of memory after it is freed
                ubusd_delete_event_source(ev);
                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Which might be a false positives, but in order to make the code pass the
static analyzer checks, rewrite the while loops on lists with the safe
list iterator.

Signed-off-by: Petr Štetiar <ynezz at true.cz>
---
 libubus-req.c | 6 ++----
 libubus.c     | 8 +++++---
 ubusd_event.c | 5 ++---
 ubusd_obj.c   | 5 ++---
 ubusd_proto.c | 6 +++---
 5 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/libubus-req.c b/libubus-req.c
index 97785a1e24cd..fd9a548839e4 100644
--- a/libubus-req.c
+++ b/libubus-req.c
@@ -40,11 +40,9 @@ static void req_data_cb(struct ubus_request *req, int type, struct blob_attr *da
 
 static void __ubus_process_req_data(struct ubus_request *req)
 {
-	struct ubus_pending_data *data;
+	struct ubus_pending_data *data, *tmp;
 
-	while (!list_empty(&req->pending)) {
-		data = list_first_entry(&req->pending,
-			struct ubus_pending_data, list);
+	list_for_each_entry_safe(data, tmp, &req->pending, list) {
 		list_del(&data->list);
 		if (!req->cancelled)
 			req_data_cb(req, data->type, data->data);
diff --git a/libubus.c b/libubus.c
index 846ae83bcc12..b405891416c2 100644
--- a/libubus.c
+++ b/libubus.c
@@ -115,10 +115,12 @@ ubus_process_msg(struct ubus_context *ctx, struct ubus_msghdr_buf *buf, int fd)
 static void ubus_process_pending_msg(struct uloop_timeout *timeout)
 {
 	struct ubus_context *ctx = container_of(timeout, struct ubus_context, pending_timer);
-	struct ubus_pending_msg *pending;
+	struct ubus_pending_msg *pending, *tmp;
+
+	list_for_each_entry_safe(pending, tmp, &ctx->pending, list) {
+		if (ctx->stack_depth)
+			break;
 
-	while (!ctx->stack_depth && !list_empty(&ctx->pending)) {
-		pending = list_first_entry(&ctx->pending, struct ubus_pending_msg, list);
 		list_del(&pending->list);
 		ubus_process_msg(ctx, &pending->hdr, -1);
 		free(pending);
diff --git a/ubusd_event.c b/ubusd_event.c
index d36bcb73ddda..ef433f81a71e 100644
--- a/ubusd_event.c
+++ b/ubusd_event.c
@@ -35,10 +35,9 @@ static void ubusd_delete_event_source(struct event_source *evs)
 
 void ubusd_event_cleanup_object(struct ubus_object *obj)
 {
-	struct event_source *ev;
+	struct event_source *ev, *tmp;
 
-	while (!list_empty(&obj->events)) {
-		ev = list_first_entry(&obj->events, struct event_source, list);
+	list_for_each_entry_safe(ev, tmp, &obj->events, list) {
 		ubusd_delete_event_source(ev);
 	}
 }
diff --git a/ubusd_obj.c b/ubusd_obj.c
index 0c9cb9ae8208..dd44882e08d5 100644
--- a/ubusd_obj.c
+++ b/ubusd_obj.c
@@ -20,13 +20,12 @@ struct avl_tree path;
 
 static void ubus_unref_object_type(struct ubus_object_type *type)
 {
-	struct ubus_method *m;
+	struct ubus_method *m, *tmp;
 
 	if (--type->refcount > 0)
 		return;
 
-	while (!list_empty(&type->methods)) {
-		m = list_first_entry(&type->methods, struct ubus_method, list);
+	list_for_each_entry_safe(m, tmp, &type->methods, list) {
 		list_del(&m->list);
 		free(m);
 	}
diff --git a/ubusd_proto.c b/ubusd_proto.c
index 2d04b5a7da6e..4dd89ddb4939 100644
--- a/ubusd_proto.c
+++ b/ubusd_proto.c
@@ -519,12 +519,12 @@ free:
 
 void ubusd_proto_free_client(struct ubus_client *cl)
 {
-	struct ubus_object *obj;
+	struct ubus_object *obj, *tmp;
 
-	while (!list_empty(&cl->objects)) {
-		obj = list_first_entry(&cl->objects, struct ubus_object, list);
+	list_for_each_entry_safe(obj, tmp, &cl->objects, list) {
 		ubusd_free_object(obj);
 	}
+
 	ubus_msg_free(cl->retmsg);
 	blob_buf_free(&cl->b);
 

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel