Re: [PATCH v2 2/8] liveupdate: Protect FLB lists with rwsem

[Samiullah Khawaja <skhawaja@google.com>]

On Wed, Mar 18, 2026 at 10:16:40AM -0400, Pasha Tatashin wrote:
>Because liveupdate FLB objects will soon drop their persistent module
>references when registered, list traversals must be protected against
>concurrent module unloading.
>
>Introduce two read-write semaphores to provide this protection:
>1. A global luo_flb_lock protects the global registry of FLBs.
>2. A per-handler flb_lock protects the handler's specific list of FLB
>   dependencies.
>
>Read locks are used during concurrent list traversals (e.g., during
>preservation and serialization). Write locks are taken during registration
>and unregistration. When both locks are required, the global luo_flb_lock
>is strictly acquired before the per-handler flb_lock to prevent deadlocks.
>
>Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
>---
> include/linux/liveupdate.h   |  3 +++
> kernel/liveupdate/luo_file.c |  1 +
> kernel/liveupdate/luo_flb.c  | 16 ++++++++++++++++
> 3 files changed, 20 insertions(+)
>
>diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
>index dd11fdc76a5f..8394fb2d8774 100644
>--- a/include/linux/liveupdate.h
>+++ b/include/linux/liveupdate.h
>@@ -12,6 +12,7 @@
> #include <linux/kho/abi/luo.h>
> #include <linux/list.h>
> #include <linux/mutex.h>
>+#include <linux/rwsem.h>
> #include <linux/types.h>
> #include <uapi/linux/liveupdate.h>
>
>@@ -107,6 +108,8 @@ struct liveupdate_file_handler {
> 	struct list_head __private list;
> 	/* A list of FLB dependencies. */
> 	struct list_head __private flb_list;
>+	/* Protects flb_list */
>+	struct rw_semaphore __private flb_lock;
> };
>
> /**
>diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
>index 6a0ae29c6a24..96fdd5790dcc 100644
>--- a/kernel/liveupdate/luo_file.c
>+++ b/kernel/liveupdate/luo_file.c
>@@ -873,6 +873,7 @@ int liveupdate_register_file_handler(struct liveupdate_file_handler *fh)
> 		}
>
> 		INIT_LIST_HEAD(&ACCESS_PRIVATE(fh, flb_list));
>+		init_rwsem(&ACCESS_PRIVATE(fh, flb_lock));
> 		INIT_LIST_HEAD(&ACCESS_PRIVATE(fh, list));
> 		list_add_tail(&ACCESS_PRIVATE(fh, list), &luo_file_handler_list);
> 	}
>diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
>index f52e8114837e..91910d806d1d 100644
>--- a/kernel/liveupdate/luo_flb.c
>+++ b/kernel/liveupdate/luo_flb.c
>@@ -49,6 +49,7 @@
> #include <linux/liveupdate.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
>+#include <linux/rwsem.h>
> #include <linux/slab.h>
> #include <linux/unaligned.h>
> #include "luo_internal.h"
>@@ -70,6 +71,7 @@ struct luo_flb_global {
> 	long count;
> };
>
>+static DECLARE_RWSEM(luo_flb_lock);
> static struct luo_flb_global luo_flb_global = {
> 	.list = LIST_HEAD_INIT(luo_flb_global.list),
> };
>@@ -240,6 +242,8 @@ int luo_flb_file_preserve(struct liveupdate_file_handler *fh)
> 	struct luo_flb_link *iter;
> 	int err = 0;
>
>+	guard(rwsem_read)(&ACCESS_PRIVATE(fh, flb_lock));
>+
> 	list_for_each_entry(iter, flb_list, list) {
> 		err = luo_flb_file_preserve_one(iter->flb);
> 		if (err)
>@@ -272,6 +276,8 @@ void luo_flb_file_unpreserve(struct liveupdate_file_handler *fh)
> 	struct list_head *flb_list = &ACCESS_PRIVATE(fh, flb_list);
> 	struct luo_flb_link *iter;
>
>+	guard(rwsem_read)(&ACCESS_PRIVATE(fh, flb_lock));
>+
> 	list_for_each_entry_reverse(iter, flb_list, list)
> 		luo_flb_file_unpreserve_one(iter->flb);
> }
>@@ -292,6 +298,8 @@ void luo_flb_file_finish(struct liveupdate_file_handler *fh)
> 	struct list_head *flb_list = &ACCESS_PRIVATE(fh, flb_list);
> 	struct luo_flb_link *iter;
>
>+	guard(rwsem_read)(&ACCESS_PRIVATE(fh, flb_lock));
>+
> 	list_for_each_entry_reverse(iter, flb_list, list)
> 		luo_flb_file_finish_one(iter->flb);
> }
>@@ -355,6 +363,9 @@ int liveupdate_register_flb(struct liveupdate_file_handler *fh,
> 	if (!luo_session_quiesce())
> 		return -EBUSY;
>
>+	guard(rwsem_write)(&luo_flb_lock);
>+	guard(rwsem_write)(&ACCESS_PRIVATE(fh, flb_lock));

Since FLBs are linked with file handlers and the file_handler can be
unregistered/registered while this is running, should the luo_file_lock
write be taken here? I think maybe we don't need a separate luo_flb_lock
and the luo_file_lock should provide enough protection if we acquire it
here, as a file_handler is supposed to be registered first and then flb
needs to be registered against it?

Maybe we can have one luo_register_lock?
>+
> 	/* Check that this FLB is not already linked to this file handler */
> 	err = -EEXIST;
> 	list_for_each_entry(iter, flb_list, list) {
>@@ -444,6 +455,9 @@ int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> 	if (!luo_session_quiesce())
> 		return -EBUSY;
>
>+	guard(rwsem_write)(&luo_flb_lock);
>+	guard(rwsem_write)(&ACCESS_PRIVATE(fh, flb_lock));
>+
> 	/* Find and remove the link from the file handler's list */
> 	list_for_each_entry(iter, flb_list, list) {
> 		if (iter->flb == flb) {
>@@ -638,6 +652,8 @@ void luo_flb_serialize(void)
> 	struct liveupdate_flb *gflb;
> 	int i = 0;
>
>+	guard(rwsem_read)(&luo_flb_lock);
>+
> 	list_private_for_each_entry(gflb, &luo_flb_global.list, private.list) {
> 		struct luo_flb_private *private = luo_flb_get_private(gflb);
>
>-- 
>2.53.0.851.ga537e3e6e9-goog
>
>