[PATCH v3 5/5] liveupdate: harden FLB lifetime and teardown paths

[Oskar Gerlicz Kowalczuk <oskar@gerlicz.space>]

The remaining FLB teardown paths still had a few lifetime holes. Incoming
counters could underflow, the returned incoming object was not pinned
against concurrent finish, and copied incoming FLB metadata could be
freed while another thread was still scanning it.

That makes incoming FLB teardown racy and can leave callers with stale
objects or freed metadata.

Guard the FLB counters, make liveupdate_flb_get_incoming() hold the
incoming FLB lock until the caller releases it, and serialize access to
the copied incoming FLB metadata while it is being retrieved or
discarded.

Fixes: 8c31961065ee ("liveupdate: harden FLB and incoming teardown paths")
Signed-off-by: Oskar Gerlicz Kowalczuk <oskar@gerlicz.space>
---
 include/linux/liveupdate.h       |  5 ++
 kernel/liveupdate/luo_flb.c      | 78 ++++++++++++++++++++++++--------
 kernel/liveupdate/luo_internal.h |  1 +
 lib/tests/liveupdate.c           |  2 +
 4 files changed, 66 insertions(+), 20 deletions(-)

diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
index 611907f57127..dd34587a9c4e 100644
--- a/include/linux/liveupdate.h
+++ b/include/linux/liveupdate.h
@@ -240,6 +240,7 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
 			      struct liveupdate_flb *flb);
 
 int liveupdate_flb_get_incoming(struct liveupdate_flb *flb, void **objp);
+void liveupdate_flb_put_incoming(struct liveupdate_flb *flb);
 int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb, void **objp);
 
 #else /* CONFIG_LIVEUPDATE */
@@ -286,6 +287,10 @@ static inline int liveupdate_flb_get_incoming(struct liveupdate_flb *flb,
 	return -EOPNOTSUPP;
 }
 
+static inline void liveupdate_flb_put_incoming(struct liveupdate_flb *flb)
+{
+}
+
 static inline int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb,
 					      void **objp)
 {
diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
index cdd293408138..2cd5864c8b26 100644
--- a/kernel/liveupdate/luo_flb.c
+++ b/kernel/liveupdate/luo_flb.c
@@ -70,6 +70,8 @@ struct luo_flb_global {
 	long count;
 };
 
+static DEFINE_MUTEX(luo_flb_incoming_lock);
+
 static struct luo_flb_global luo_flb_global = {
 	.list = LIST_HEAD_INIT(luo_flb_global.list),
 };
@@ -128,6 +130,9 @@ static void luo_flb_file_unpreserve_one(struct liveupdate_flb *flb)
 	struct luo_flb_private *private = luo_flb_get_private(flb);
 
 	scoped_guard(mutex, &private->outgoing.lock) {
+		if (WARN_ON_ONCE(!private->outgoing.count))
+			return;
+
 		private->outgoing.count--;
 		if (!private->outgoing.count) {
 			struct liveupdate_flb_op_args args = {0};
@@ -161,22 +166,24 @@ static int luo_flb_retrieve_one(struct liveupdate_flb *flb)
 	if (private->incoming.retrieved)
 		return 0;
 
-	if (!fh->active)
-		return -ENODATA;
-
-	for (int i = 0; i < fh->header_ser->count; i++) {
-		if (strnlen(fh->ser[i].name, sizeof(fh->ser[i].name)) ==
-		    sizeof(fh->ser[i].name))
-			return -EINVAL;
+	scoped_guard(mutex, &luo_flb_incoming_lock) {
+		if (!fh->active)
+			return -ENODATA;
 
-		if (!strcmp(fh->ser[i].name, flb->compatible)) {
-			if (!fh->ser[i].count)
+		for (int i = 0; i < fh->header_ser->count; i++) {
+			if (strnlen(fh->ser[i].name, sizeof(fh->ser[i].name)) ==
+			    sizeof(fh->ser[i].name))
 				return -EINVAL;
 
-			private->incoming.data = fh->ser[i].data;
-			private->incoming.count = fh->ser[i].count;
-			found = true;
-			break;
+			if (!strcmp(fh->ser[i].name, flb->compatible)) {
+				if (!fh->ser[i].count)
+					return -EINVAL;
+
+				private->incoming.data = fh->ser[i].data;
+				private->incoming.count = fh->ser[i].count;
+				found = true;
+				break;
+			}
 		}
 	}
 
@@ -201,8 +208,12 @@ static void luo_flb_file_finish_one(struct liveupdate_flb *flb)
 	struct luo_flb_private *private = luo_flb_get_private(flb);
 	u64 count;
 
-	scoped_guard(mutex, &private->incoming.lock)
+	scoped_guard(mutex, &private->incoming.lock) {
+		if (WARN_ON_ONCE(!private->incoming.count))
+			return;
+
 		count = --private->incoming.count;
+	}
 
 	if (!count) {
 		struct liveupdate_flb_op_args args = {0};
@@ -303,6 +314,17 @@ void luo_flb_file_finish(struct liveupdate_file_handler *fh)
 		luo_flb_file_finish_one(iter->flb);
 }
 
+void luo_flb_discard_incoming(void)
+{
+	struct luo_flb_header *fh = &luo_flb_global.incoming;
+
+	guard(mutex)(&luo_flb_incoming_lock);
+	kfree(fh->header_ser);
+	fh->header_ser = NULL;
+	fh->ser = NULL;
+	fh->active = false;
+}
+
 /**
  * liveupdate_register_flb - Associate an FLB with a file handler and register it globally.
  * @fh:   The file handler that will now depend on the FLB.
@@ -490,7 +512,8 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
  * @objp: Output parameter; will be populated with the live shared object.
  *
  * Returns a pointer to its shared live object for the incoming (post-reboot)
- * path.
+ * path. The returned pointer remains valid until
+ * liveupdate_flb_put_incoming() is called.
  *
  * If this is the first time the object is requested in the new kernel, this
  * function will trigger the FLB's .retrieve() callback to reconstruct the
@@ -508,17 +531,30 @@ int liveupdate_flb_get_incoming(struct liveupdate_flb *flb, void **objp)
 	if (!liveupdate_enabled())
 		return -EOPNOTSUPP;
 
-	if (!private->incoming.obj) {
+	if (!READ_ONCE(private->incoming.obj)) {
 		int err = luo_flb_retrieve_one(flb);
 
 		if (err)
 			return err;
 	}
 
-	guard(mutex)(&private->incoming.lock);
+	mutex_lock(&private->incoming.lock);
+	if (!private->incoming.obj)
+		goto err_unlock;
 	*objp = private->incoming.obj;
 
 	return 0;
+
+err_unlock:
+	mutex_unlock(&private->incoming.lock);
+	return -ENODATA;
+}
+
+void liveupdate_flb_put_incoming(struct liveupdate_flb *flb)
+{
+	struct luo_flb_private *private = luo_flb_get_private(flb);
+
+	mutex_unlock(&private->incoming.lock);
 }
 
 /**
@@ -640,9 +676,11 @@ int __init luo_flb_setup_incoming(void *fdt_in)
 	if (!header_copy)
 		return -ENOMEM;
 
-	luo_flb_global.incoming.header_ser = header_copy;
-	luo_flb_global.incoming.ser = (void *)(header_copy + 1);
-	luo_flb_global.incoming.active = true;
+	scoped_guard(mutex, &luo_flb_incoming_lock) {
+		luo_flb_global.incoming.header_ser = header_copy;
+		luo_flb_global.incoming.ser = (void *)(header_copy + 1);
+		luo_flb_global.incoming.active = true;
+	}
 
 	return 0;
 }
diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h
index 4842c7dbeb63..e4d1919370f1 100644
--- a/kernel/liveupdate/luo_internal.h
+++ b/kernel/liveupdate/luo_internal.h
@@ -113,6 +113,7 @@ void luo_flb_file_unpreserve(struct liveupdate_file_handler *fh);
 void luo_flb_file_finish(struct liveupdate_file_handler *fh);
 int __init luo_flb_setup_outgoing(void *fdt);
 int __init luo_flb_setup_incoming(void *fdt);
+void luo_flb_discard_incoming(void);
 void luo_flb_serialize(void);
 
 #ifdef CONFIG_LIVEUPDATE_TEST
diff --git a/lib/tests/liveupdate.c b/lib/tests/liveupdate.c
index 496d6ef91a30..620cefd52229 100644
--- a/lib/tests/liveupdate.c
+++ b/lib/tests/liveupdate.c
@@ -105,6 +105,8 @@ static void liveupdate_test_init(void)
 			pr_err("liveupdate_flb_get_incoming for %s failed: %pe\n",
 			       flb->compatible, ERR_PTR(err));
 		}
+		if (!err)
+			liveupdate_flb_put_incoming(flb);
 	}
 	initialized = true;
 }
-- 
2.53.0