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

Pasha Tatashin posted 8 patches 2 weeks, 5 days ago
There is a newer version of this series
[PATCH v2 2/8] liveupdate: Protect FLB lists with rwsem
Posted by Pasha Tatashin 2 weeks, 5 days ago
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));
+
 	/* 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
Re: [PATCH v2 2/8] liveupdate: Protect FLB lists with rwsem
Posted by Samiullah Khawaja 2 weeks, 3 days ago
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
>
>
Re: [PATCH v2 2/8] liveupdate: Protect FLB lists with rwsem
Posted by Pasha Tatashin 2 weeks, 3 days ago
> >@@ -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

Hi Sami,

Yes, thank you for catching this. I thought we were safe since we are
taking fh->flb_lock, but you are correct. I am going to do what you
suggested below and use a single luo_registration_lock for both FLB
and File Handler registrations. Reads will also be shared; this will
simplify locking.

Pasha

> 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
> >
> >