[Xen-devel] [PATCH for-4.9 v2] xen/livepatch: Don't crash on encountering STN_UNDEF relocations

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

A symndx of STN_UNDEF is special, and means a symbol value of 0.  While
legitimate in the ELF standard, its existance in a livepatch is questionable
at best.  Until a plausible usecase presents itself, reject such a relocation
with -EOPNOTSUPP.

Additionally, perform a safety check on elf->sym[symndx].sym before
derefencing it, to avoid tripping over a NULL pointer when calculating val.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Julien Grall <julien.grall@xxxxxxx>

v2:
 * Reject STN_UNDEF with -EOPNOTSUPP
---
 xen/arch/arm/arm32/livepatch.c | 17 +++++++++++++++--
 xen/arch/arm/arm64/livepatch.c | 17 +++++++++++++++--
 xen/arch/x86/livepatch.c       | 17 +++++++++++++++--
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index a328179..53fee91 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -254,14 +254,27 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
             addend = get_addend(type, dest);
         }
 
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+
         if ( symndx > elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative symbol wants symbol@%u 
which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
-
-        val = elf->sym[symndx].sym->st_value; /* S */
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+        else
+            val = elf->sym[symndx].sym->st_value; /* S */
 
         rc = perform_rel(type, dest, val, addend);
         switch ( rc )
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 63929b1..b033763 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -252,14 +252,27 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
         int ovf = 0;
         uint64_t val;
 
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+
         if ( symndx > elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants 
symbol@%u which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
-
-        val = elf->sym[symndx].sym->st_value +  r->r_addend; /* S+A */
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+        else
+            val = elf->sym[symndx].sym->st_value + r->r_addend; /* S+A */
 
         /* ARM64 operations at minimum are always 32-bit. */
         if ( r->r_offset >= base->sec->sh_size ||
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 7917610..bfa576c 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -170,14 +170,27 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
         uint8_t *dest = base->load_addr + r->r_offset;
         uint64_t val;
 
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+
         if ( symndx > elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants 
symbol@%u which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
-
-        val = r->r_addend + elf->sym[symndx].sym->st_value;
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+        else
+            val = r->r_addend + elf->sym[symndx].sym->st_value;
 
         switch ( ELF64_R_TYPE(r->r_info) )
         {
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel