diff mbox series

[2/5] xen/livepatch: search for symbols in all loaded payloads

Message ID 20231130142944.46322-3-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen/livepatch: fixes for the pre-apply / post-revert hooks | expand

Commit Message

Roger Pau Monne Nov. 30, 2023, 2:29 p.m. UTC
When checking if an address belongs to a patch, or when resolving a symbol,
take into account all loaded livepatch payloads, even if not applied.

This is required in order for the pre-apply and post-revert hooks to work
properly, or else Xen won't detect the intruction pointer belonging to those
hooks as being part of the currently active text.

Move the RCU handling to be used for payload_list instead of applied_list, as
now the calls from trap code will iterate over the payload_list.

Fixes: 8313c864fa95 ('livepatch: Implement pre-|post- apply|revert hooks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/common/livepatch.c | 49 +++++++++++++++---------------------------
 1 file changed, 17 insertions(+), 32 deletions(-)

Comments

Ross Lagerwall Feb. 23, 2024, 10:11 a.m. UTC | #1
On Thu, Nov 30, 2023 at 2:30 PM Roger Pau Monne <roger.pau@citrix.com> wrote:
>
> When checking if an address belongs to a patch, or when resolving a symbol,
> take into account all loaded livepatch payloads, even if not applied.
>
> This is required in order for the pre-apply and post-revert hooks to work
> properly, or else Xen won't detect the intruction pointer belonging to those
> hooks as being part of the currently active text.
>
> Move the RCU handling to be used for payload_list instead of applied_list, as
> now the calls from trap code will iterate over the payload_list.
>
> Fixes: 8313c864fa95 ('livepatch: Implement pre-|post- apply|revert hooks')
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff mbox series

Patch

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 3199432f11f5..4e775be66571 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -36,13 +36,14 @@ 
  * caller in schedule_work.
  */
 static DEFINE_SPINLOCK(payload_lock);
-static LIST_HEAD(payload_list);
-
 /*
- * Patches which have been applied. Need RCU in case we crash (and then
- * traps code would iterate via applied_list) when adding entries on the list.
+ * Need RCU in case we crash (and then traps code would iterate via
+ * payload_list) when adding entries on the list.
  */
-static DEFINE_RCU_READ_LOCK(rcu_applied_lock);
+static DEFINE_RCU_READ_LOCK(rcu_payload_lock);
+static LIST_HEAD(payload_list);
+
+/* Patches which have been applied. Only modified from stop machine context. */
 static LIST_HEAD(applied_list);
 
 static unsigned int payload_cnt;
@@ -111,12 +112,8 @@  bool is_patch(const void *ptr)
     const struct payload *data;
     bool r = false;
 
-    /*
-     * Only RCU locking since this list is only ever changed during apply
-     * or revert context. And in case it dies there we need an safe list.
-     */
-    rcu_read_lock(&rcu_applied_lock);
-    list_for_each_entry_rcu ( data, &applied_list, applied_list )
+    rcu_read_lock(&rcu_payload_lock);
+    list_for_each_entry_rcu ( data, &payload_list, list )
     {
         if ( (ptr >= data->rw_addr &&
               ptr < (data->rw_addr + data->rw_size)) ||
@@ -130,7 +127,7 @@  bool is_patch(const void *ptr)
         }
 
     }
-    rcu_read_unlock(&rcu_applied_lock);
+    rcu_read_unlock(&rcu_payload_lock);
 
     return r;
 }
@@ -166,12 +163,8 @@  static const char *cf_check livepatch_symbols_lookup(
     const void *va = (const void *)addr;
     const char *n = NULL;
 
-    /*
-     * Only RCU locking since this list is only ever changed during apply
-     * or revert context. And in case it dies there we need an safe list.
-     */
-    rcu_read_lock(&rcu_applied_lock);
-    list_for_each_entry_rcu ( data, &applied_list, applied_list )
+    rcu_read_lock(&rcu_payload_lock);
+    list_for_each_entry_rcu ( data, &payload_list, list )
     {
         if ( va < data->text_addr ||
              va >= (data->text_addr + data->text_size) )
@@ -200,7 +193,7 @@  static const char *cf_check livepatch_symbols_lookup(
         n = data->symtab[best].name;
         break;
     }
-    rcu_read_unlock(&rcu_applied_lock);
+    rcu_read_unlock(&rcu_payload_lock);
 
     return n;
 }
@@ -1074,7 +1067,8 @@  static void free_payload(struct payload *data)
 {
     ASSERT(spin_is_locked(&payload_lock));
     unregister_virtual_region(&data->region);
-    list_del(&data->list);
+    list_del_rcu(&data->list);
+    rcu_barrier();
     payload_cnt--;
     payload_version++;
     free_payload_data(data);
@@ -1173,7 +1167,7 @@  static int livepatch_upload(struct xen_sysctl_livepatch_upload *upload)
         INIT_LIST_HEAD(&data->list);
         INIT_LIST_HEAD(&data->applied_list);
 
-        list_add_tail(&data->list, &payload_list);
+        list_add_tail_rcu(&data->list, &payload_list);
         payload_cnt++;
         payload_version++;
     }
@@ -1384,11 +1378,7 @@  static int apply_payload(struct payload *data)
 
 static inline void apply_payload_tail(struct payload *data)
 {
-    /*
-     * We need RCU variant (which has barriers) in case we crash here.
-     * The applied_list is iterated by the trap code.
-     */
-    list_add_tail_rcu(&data->applied_list, &applied_list);
+    list_add_tail(&data->applied_list, &applied_list);
 
     data->state = LIVEPATCH_STATE_APPLIED;
 }
@@ -1428,12 +1418,7 @@  static int revert_payload(struct payload *data)
 
 static inline void revert_payload_tail(struct payload *data)
 {
-
-    /*
-     * We need RCU variant (which has barriers) in case we crash here.
-     * The applied_list is iterated by the trap code.
-     */
-    list_del_rcu(&data->applied_list);
+    list_del(&data->applied_list);
 
     data->reverted = true;
     data->state = LIVEPATCH_STATE_CHECKED;