Re: [PATCH v3 09/10] liveupdate: Make unregister functions return void

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On Thu, Mar 26, 2026 at 11:33 PM Pasha Tatashin
<pasha.tatashin@soleen.com> wrote:
>
> Change liveupdate_unregister_file_handler and liveupdate_unregister_flb
> to return void instead of an error code. This follows the design
> principle that unregistration during module unload should not fail,
> as the unload cannot be stopped at that point.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  include/linux/liveupdate.h   | 14 ++++++--------
>  kernel/liveupdate/luo_file.c | 14 ++------------
>  kernel/liveupdate/luo_flb.c  | 11 +++--------
>  3 files changed, 11 insertions(+), 28 deletions(-)
>
> diff --git a/include/linux/liveupdate.h b/include/linux/liveupdate.h
> index 73ca84de3eae..2ae27711ac41 100644
> --- a/include/linux/liveupdate.h
> +++ b/include/linux/liveupdate.h
> @@ -229,12 +229,12 @@ bool liveupdate_enabled(void);
>  int liveupdate_reboot(void);
>
>  int liveupdate_register_file_handler(struct liveupdate_file_handler *fh);
> -int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh);
> +void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh);
>
>  int liveupdate_register_flb(struct liveupdate_file_handler *fh,
>                             struct liveupdate_flb *flb);
> -int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> -                             struct liveupdate_flb *flb);
> +void liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> +                              struct liveupdate_flb *flb);
>
>  int liveupdate_flb_get_incoming(struct liveupdate_flb *flb, void **objp);
>  int liveupdate_flb_get_outgoing(struct liveupdate_flb *flb, void **objp);
> @@ -256,9 +256,8 @@ static inline int liveupdate_register_file_handler(struct liveupdate_file_handle
>         return -EOPNOTSUPP;
>  }
>
> -static inline int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
> +static inline void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
>  {
> -       return -EOPNOTSUPP;
>  }
>
>  static inline int liveupdate_register_flb(struct liveupdate_file_handler *fh,
> @@ -267,10 +266,9 @@ static inline int liveupdate_register_flb(struct liveupdate_file_handler *fh,
>         return -EOPNOTSUPP;
>  }
>
> -static inline int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> -                                           struct liveupdate_flb *flb)
> +static inline void liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> +                                            struct liveupdate_flb *flb)
>  {
> -       return -EOPNOTSUPP;
>  }
>
>  static inline int liveupdate_flb_get_incoming(struct liveupdate_flb *flb,
> diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c
> index dd55e5e74d69..761e8f7cfc82 100644
> --- a/kernel/liveupdate/luo_file.c
> +++ b/kernel/liveupdate/luo_file.c
> @@ -884,25 +884,15 @@ int liveupdate_register_file_handler(struct liveupdate_file_handler *fh)
>   *
>   * Unregisters the file handler from the liveupdate core. This function
>   * reverses the operations of liveupdate_register_file_handler().
> - *
> - * It ensures safe removal by checking that:
> - * No FLB registered with this file handler.
> - *
> - * If the unregistration fails, the internal test state is reverted.
> - *
> - * Return: 0 Success. -EOPNOTSUPP when live update is not enabled. -EBUSY A live
> - * update is in progress, FLB is registred with this file handler.
>   */
> -int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
> +void liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh)
>  {
>         if (!liveupdate_enabled())
> -               return -EOPNOTSUPP;
> +               return;
>
>         guard(rwsem_write)(&luo_register_rwlock);
>         luo_flb_unregister_all(fh);
>         list_del(&ACCESS_PRIVATE(fh, list));
>
>         module_put(fh->ops->owner);
> -
> -       return 0;
>  }
> diff --git a/kernel/liveupdate/luo_flb.c b/kernel/liveupdate/luo_flb.c
> index f8348138de70..8f3c9c63e320 100644
> --- a/kernel/liveupdate/luo_flb.c
> +++ b/kernel/liveupdate/luo_flb.c
> @@ -492,21 +492,16 @@ int liveupdate_register_flb(struct liveupdate_file_handler *fh,
>   * owner module (acquired during registration) is released.
>   *
>   * Context: It is typically called from a subsystem's module exit function.
> - * Return: 0 on success.
> - *         -EOPNOTSUPP if live update is disabled.
> - *         -ENOENT if the FLB was not found in the file handler's list.
>   */
> -int liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> -                             struct liveupdate_flb *flb)
> +void liveupdate_unregister_flb(struct liveupdate_file_handler *fh,
> +                              struct liveupdate_flb *flb)
>  {
>         if (!liveupdate_enabled())
> -               return -EOPNOTSUPP;
> +               return;
>
>         guard(rwsem_write)(&luo_register_rwlock);
>
>         luo_flb_unregister_one(fh, flb);
> -
> -       return 0;
>  }

A question from Sashiko:
If the file handler is unregistered and freed by its owner, its dependent
FLBs might still retain a stale pointer to it. When an FLB is later
unregistered and calls liveupdate_unregister_flb() with this stale fh
pointer, could luo_flb_unregister_one() dereference the freed memory and
cause a use-after-free?

No, there is no UAF possability here.

1. FLBs do not keep pointers to file handlers.
The relationship between file handlers and FLBs is asymmetric. A
struct liveupdate_file_handler maintains a list of its dependent FLBs
in its flb_list. And, the struct liveupdate_flb only tracks the number
of handlers using it and its position in a global list. It does not
store pointers back to the file handlers.

2. Manual FLB unregistration: When a subsystem calls
liveupdate_unregister_flb, it is responsible for providing a valid fh
pointer. Since the subsystem typically owns the lifetime of the fh (a
static structure in the module), it is the caller's responsibility not
to pass a stale pointer.

3. Atomatic FLB unregistration during
liveupdate_unregister_file_handler function, explicitly calls
luo_flb_unregister_all(fh): luo_flb_unregister_all(fh) iterates
through the handler's flb_list, calls luo_flb_unregister_one for each,
which removes the luo_flb_link from the handler and frees it. This
ensures that when the handler is gone, all its associations with FLBs
are already destroyed.

Pasha