xen/livepatch: Don't crash on encountering STN_UNDEF relocations

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, fix an off-by-one error while range checking symndx, and 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@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> [x86 and arm32]
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>

diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index a328179b43795ad7562fe127525a06dd33199168..41378a54aefcd6e0a11e6037320d609d82085356 100644 (file)
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -254,12 +254,24 @@ int arch_livepatch_perform(struct livepatch_elf *elf,
             addend = get_addend(type, dest);
         }
 
-        if ( symndx > elf->nsym )
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+        else if ( symndx >= elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative symbol wants symbol@%u which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
 
         val = elf->sym[symndx].sym->st_value; /* S */
 

diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 63929b17518b3e6c30851b094740e19f89cb2526..2247b925a05fcb9fd29b55e5a37d52a31177f964 100644 (file)
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -252,12 +252,24 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
         int ovf = 0;
         uint64_t val;
 
-        if ( symndx > elf->nsym )
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+        else if ( symndx >= elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants symbol@%u which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No relative symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
 
         val = elf->sym[symndx].sym->st_value +  r->r_addend; /* S+A */
 

diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 7917610f1dceacb1a834a3e788cfb41f4345a36f..406eb910cc8fd3bdb1064348890d2bf6936746d3 100644 (file)
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -170,12 +170,24 @@ int arch_livepatch_perform_rela(struct livepatch_elf *elf,
         uint8_t *dest = base->load_addr + r->r_offset;
         uint64_t val;
 
-        if ( symndx > elf->nsym )
+        if ( symndx == STN_UNDEF )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Encountered STN_UNDEF\n",
+                    elf->name);
+            return -EOPNOTSUPP;
+        }
+        else if ( symndx >= elf->nsym )
         {
             dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants symbol@%u which is past end!\n",
                     elf->name, symndx);
             return -EINVAL;
         }
+        else if ( !elf->sym[symndx].sym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: No symbol@%u\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
 
         val = r->r_addend + elf->sym[symndx].sym->st_value;