[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH v10 6/6] xen: use DEFINE_SYMBOL as required



Use DEFINE_SYMBOL and the two static inline functions that come with it
for comparisons and subtractions of:

_start, _end, _stext, _etext, _srodata, _erodata, _sinittext,
_einittext

Use explicit casts to uintptr_t when it is not possible to use the
provided static inline functions.

M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
pointers that address elements of the same array

Since we are changing the body of is_kernel_text and friends, take the
opportunity to remove the leading underscores in the local variables
names, which are violationg namespace rules. Also make the local p__
variable const.

https://wiki.sei.cmu.edu/confluence/display/c/ARR36-C.+Do+not+subtract+or+compare+two+pointers+that+do+not+refer+to+the+same+array

QAVerify: 2761
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
CC: JBeulich@xxxxxxxx
CC: andrew.cooper3@xxxxxxxxxx
---
Changes in v10:
- new patch
---
 xen/arch/arm/alternative.c        |  5 +++--
 xen/arch/arm/arm32/livepatch.c    | 14 +++++++++++++-
 xen/arch/arm/arm64/livepatch.c    | 14 +++++++++++++-
 xen/arch/arm/livepatch.c          | 16 ++++++++++++++--
 xen/arch/arm/mm.c                 |  4 ++--
 xen/arch/arm/setup.c              |  5 +++--
 xen/arch/x86/setup.c              |  6 ++++--
 xen/include/asm-arm/grant_table.h |  3 ++-
 xen/include/xen/kernel.h          | 37 +++++++++++++++++++++----------------
 9 files changed, 75 insertions(+), 29 deletions(-)

diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
index cef9cca..7a1fe5a 100644
--- a/xen/arch/arm/alternative.c
+++ b/xen/arch/arm/alternative.c
@@ -192,7 +192,7 @@ static int __apply_alternatives_multi_stop(void *unused)
         int ret;
         struct alt_region region;
         mfn_t xen_mfn = virt_to_mfn(_start);
-        paddr_t xen_size = _end - _start;
+        paddr_t xen_size = xen_diff(_start, _end);
         unsigned int xen_order = get_order_from_bytes(xen_size);
         void *xenmap;
 
@@ -210,7 +210,8 @@ static int __apply_alternatives_multi_stop(void *unused)
         region.begin = __alt_instructions;
         region.end = (struct alt_instr *)__alt_instructions_end;
 
-        ret = __apply_alternatives(&region, xenmap - (void *)_start);
+        ret = __apply_alternatives(&region, (uintptr_t)xenmap -
+                                            (uintptr_t)_start);
         /* The patching is not expected to fail during boot. */
         BUG_ON(ret != 0);
 
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 41378a5..5f7390e 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -56,7 +56,19 @@ void arch_livepatch_apply(struct livepatch_func *func)
     else
         insn = 0xe1a00000; /* mov r0, r0 */
 
-    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    /*
+     * We need to calculate the offset of the address from _start, and
+     * apply that to our own map, to find where we have this mapped.
+     * Doing these kind of games directly with pointers is contrary to
+     * the C rules for what pointers may be compared and computed.  So
+     * we do the offset calculation with integers, which is always
+     * legal.  The subsequent addition of the offset to the
+     * vmap_of_xen_text pointer is legal because the computed pointer is
+     * indeed a valid part of the object referred to by vmap_of_xen_text
+     * - namely, the byte array of our mapping of the Xen text.
+     */
+    new_ptr = ((uintptr_t)func->old_addr, - (uintptr_t)_start) +
+              vmap_of_xen_text;
     len = len / sizeof(uint32_t);
 
     /* PATCH! */
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 2247b92..362235c 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -43,7 +43,19 @@ void arch_livepatch_apply(struct livepatch_func *func)
     /* Verified in livepatch_verify_distance. */
     ASSERT(insn != AARCH64_BREAK_FAULT);
 
-    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    /*
+     * We need to calculate the offset of the address from _start, and
+     * apply that to our own map, to find where we have this mapped.
+     * Doing these kind of games directly with pointers is contrary to
+     * the C rules for what pointers may be compared and computed.  So
+     * we do the offset calculation with integers, which is always
+     * legal.  The subsequent addition of the offset to the
+     * vmap_of_xen_text pointer is legal because the computed pointer is
+     * indeed a valid part of the object referred to by vmap_of_xen_text
+     * - namely, the byte array of our mapping of the Xen text.
+     */
+    new_ptr = ((uintptr_t)func->old_addr, - (uintptr_t)_start) +
+              vmap_of_xen_text;
     len = len / sizeof(uint32_t);
 
     /* PATCH! */
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 279d52c..af4411a 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -27,7 +27,7 @@ int arch_livepatch_quiesce(void)
         return -EINVAL;
 
     text_mfn = virt_to_mfn(_start);
-    text_order = get_order_from_bytes(_end - _start);
+    text_order = get_order_from_bytes(xen_diff(_start, _end));
 
     /*
      * The text section is read-only. So re-map Xen to be able to patch
@@ -78,7 +78,19 @@ void arch_livepatch_revert(const struct livepatch_func *func)
     uint32_t *new_ptr;
     unsigned int len;
 
-    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    /*
+     * We need to calculate the offset of the address from _start, and
+     * apply that to our own map, to find where we have this mapped.
+     * Doing these kind of games directly with pointers is contrary to
+     * the C rules for what pointers may be compared and computed.  So
+     * we do the offset calculation with integers, which is always
+     * legal.  The subsequent addition of the offset to the
+     * vmap_of_xen_text pointer is legal because the computed pointer is
+     * indeed a valid part of the object referred to by vmap_of_xen_text
+     * - namely, the byte array of our mapping of the Xen text.
+     */
+    new_ptr = ((uintptr_t)func->old_addr, - (uintptr_t)_start) +
+              vmap_of_xen_text;
 
     len = livepatch_insn_len(func);
     memcpy(new_ptr, func->opaque, len);
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 9f31e81..7f9a309 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1084,8 +1084,8 @@ static void set_pte_flags_on_range(const char *p, 
unsigned long l, enum mg mg)
     ASSERT(!((unsigned long) p & ~PAGE_MASK));
     ASSERT(!(l & ~PAGE_MASK));
 
-    for ( i = (p - _start) / PAGE_SIZE; 
-          i < (p + l - _start) / PAGE_SIZE; 
+    for ( i = ((uintptr_t)p - (uintptr_t)_start) / PAGE_SIZE;
+          i < ((uintptr_t)p + l - (uintptr_t)_start) / PAGE_SIZE;
           i++ )
     {
         pte = xen_xenmap[i];
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 444857a..8d43943 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -772,8 +772,9 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     /* Register Xen's load address as a boot module. */
     xen_bootmodule = add_boot_module(BOOTMOD_XEN,
-                             (paddr_t)(uintptr_t)(_start + boot_phys_offset),
-                             (paddr_t)(uintptr_t)(_end - _start + 1), false);
+                             (paddr_t)(_start + boot_phys_offset),
+                             (paddr_t)(xen_diff(_start, _end) + 1),
+                             false);
     BUG_ON(!xen_bootmodule);
 
     setup_pagetables(boot_phys_offset);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b024339..7ede826 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -975,7 +975,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
          * respective reserve_e820_ram() invocation below.
          */
         mod[mbi->mods_count].mod_start = virt_to_mfn(_stext);
-        mod[mbi->mods_count].mod_end = __2M_rwdata_end - _stext;
+        mod[mbi->mods_count].mod_end = (uintptr_t)__2M_rwdata_end -
+                                       (uintptr_t) _stext;
     }
 
     modules_headroom = bzimage_headroom(bootstrap_map(mod), mod->mod_end);
@@ -1070,7 +1071,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
              * data until after we have switched to the relocated pagetables!
              */
             barrier();
-            move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET, _end - _start, 1);
+            move_memory(e + XEN_IMG_OFFSET, XEN_IMG_OFFSET,
+                        xen_diff(_start, _end), 1);
 
             /* Walk initial pagetables, relocating page directory entries. */
             pl4e = __va(__pa(idle_pg_table));
diff --git a/xen/include/asm-arm/grant_table.h 
b/xen/include/asm-arm/grant_table.h
index 816e3c6..1097791 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -31,7 +31,8 @@ void gnttab_mark_dirty(struct domain *d, mfn_t mfn);
  * enough space for a large grant table
  */
 #define gnttab_dom0_frames()                                             \
-    min_t(unsigned int, opt_max_grant_frames, PFN_DOWN(_etext - _stext))
+    min_t(unsigned int, opt_max_grant_frames,                            \
+          PFN_DOWN(text_diff(_stext, _etext)))
 
 #define gnttab_init_arch(gt)                                             \
 ({                                                                       \
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 548b64d..15ace6d 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -65,28 +65,33 @@
        1;                                      \
 })
 
-extern char _start[], _end[], start[];
-#define is_kernel(p) ({                         \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _start) && (__p < _end);            \
+extern char start[];
+DEFINE_SYMBOL(char, xen, _start, _end);
+#define is_kernel(p) ({                                             \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    ((uintptr_t)p__ >= (uintptr_t)_start &&                         \
+    xen_lt(p__, _end));                                             \
 })
 
-extern char _stext[], _etext[];
-#define is_kernel_text(p) ({                    \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _stext) && (__p < _etext);          \
+DEFINE_SYMBOL(char, text, _stext, _etext);
+#define is_kernel_text(p) ({                                        \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    ((uintptr_t)p__ >= (uintptr_t) _stext &&                        \
+    text_lt(p__, _etext));                                          \
 })
 
-extern const char _srodata[], _erodata[];
-#define is_kernel_rodata(p) ({                  \
-    const char *__p = (const char *)(unsigned long)(p);     \
-    (__p >= _srodata) && (__p < _erodata);      \
+DEFINE_SYMBOL(char, rodata, _srodata, _erodata);
+#define is_kernel_rodata(p) ({                                      \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    ((uintptr_t)p__ >= (uintptr_t)_srodata &&                       \
+    rodata_lt(p__, _erodata));                                      \
 })
 
-extern char _sinittext[], _einittext[];
-#define is_kernel_inittext(p) ({                \
-    char *__p = (char *)(unsigned long)(p);     \
-    (__p >= _sinittext) && (__p < _einittext);  \
+DEFINE_SYMBOL(char, inittext, _sinittext, _einittext);
+#define is_kernel_inittext(p) ({                                    \
+    const char *p__ = (const char *)(unsigned long)(p);             \
+    ((uintptr_t)p__ >= (uintptr_t) _sinittext &&                    \
+    inittext_lt(p__, _einittext));                                  \
 })
 
 extern enum system_state {
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.