diff mbox

[v1,1/9] livepatch: Bubble up sanity checks on Elf relocs

Message ID 1471216074-3007-2-git-send-email-konrad.wilk@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Aug. 14, 2016, 11:07 p.m. UTC 
The checks for RELA ELF sanity checks does not need to
be in the platform specific file and can be bubbled up
in the platform agnostic file.

This makes the ARM 32/64 implementation easier as the
duplicate checks don't have to be in the platform specific files.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v1: First submission
---
 xen/arch/x86/livepatch.c   | 12 ------------
 xen/common/livepatch_elf.c | 17 ++++++++++++++++-
 2 files changed, 16 insertions(+), 13 deletions(-)

Comments

Jan Beulich Aug. 17, 2016, 11:56 a.m. UTC | #1 
>>> On 15.08.16 at 01:07, <konrad.wilk@oracle.com> wrote:
> --- a/xen/common/livepatch_elf.c
> +++ b/xen/common/livepatch_elf.c
> @@ -365,7 +365,22 @@ int livepatch_elf_perform_relocs(struct livepatch_elf *elf)
>          }
>  
>          if ( r->sec->sh_type == SHT_RELA )
> -            rc = arch_livepatch_perform_rela(elf, base, r);
> +        {
> +            rc = 0;
> +
> +            if ( !r->sec->sh_size )
> +                continue;
> +
> +            if ( r->sec->sh_entsize < sizeof(Elf_RelA) ||
> +                 r->sec->sh_size % r->sec->sh_entsize )
> +            {
> +                dprintk(XENLOG_ERR, LIVEPATCH "%s: Section relative header is corrupted!\n",
> +                        elf->name);
> +                rc = -EINVAL;
> +            }
> +            else
> +                rc = arch_livepatch_perform_rela(elf, base, r);
> +        }
>          else /* SHT_REL */
>              rc = arch_livepatch_perform_rel(elf, base, r);

Shouldn't this be mirrored to the SHT_REL case then (with the
appropriate minor adjustments)?

Jan
diff mbox

Patch

diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index cabd0c1..06c67bc 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -132,18 +132,6 @@  int arch_livepatch_perform_rela(struct livepatch_elf *elf,
     uint64_t val;
     uint8_t *dest;
 
-    /* Nothing to do. */
-    if ( !rela->sec->sh_size )
-        return 0;
-
-    if ( rela->sec->sh_entsize < sizeof(Elf_RelA) ||
-         rela->sec->sh_size % rela->sec->sh_entsize )
-    {
-        dprintk(XENLOG_ERR, LIVEPATCH "%s: Section relative header is corrupted!\n",
-                elf->name);
-        return -EINVAL;
-    }
-
     for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
     {
         r = rela->data + i * rela->sec->sh_entsize;
diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index 789e8fc..ef1a09d 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -365,7 +365,22 @@  int livepatch_elf_perform_relocs(struct livepatch_elf *elf)
         }
 
         if ( r->sec->sh_type == SHT_RELA )
-            rc = arch_livepatch_perform_rela(elf, base, r);
+        {
+            rc = 0;
+
+            if ( !r->sec->sh_size )
+                continue;
+
+            if ( r->sec->sh_entsize < sizeof(Elf_RelA) ||
+                 r->sec->sh_size % r->sec->sh_entsize )
+            {
+                dprintk(XENLOG_ERR, LIVEPATCH "%s: Section relative header is corrupted!\n",
+                        elf->name);
+                rc = -EINVAL;
+            }
+            else
+                rc = arch_livepatch_perform_rela(elf, base, r);
+        }
         else /* SHT_REL */
             rc = arch_livepatch_perform_rel(elf, base, r);