Re: [Xen-devel] [PATCH v4 1/9] livepatch: Clear .bss when payload is reverted

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Aug 25, 2016 at 05:08:16PM +0100, Andrew Cooper wrote:
> On 24/08/16 09:55, Jan Beulich wrote:
> > > > > On 24.08.16 at 04:22, <konrad.wilk@xxxxxxxxxx> wrote:
> > > --- a/xen/common/livepatch.c
> > > +++ b/xen/common/livepatch.c
> > > @@ -70,6 +70,9 @@ struct payload {
> > >       unsigned int nsyms;                  /* Nr of entries in .strtab 
> > > and symbols. */
> > >       struct livepatch_build_id id;        /* 
> > > ELFNOTE_DESC(.note.gnu.build-id) of the payload. */
> > >       struct livepatch_build_id dep;       /* 
> > > ELFNOTE_DESC(.livepatch.depends). */
> > > +    void **bss;                          /* .bss's of the payload. */
> > > +    size_t *bss_size;                    /* and their sizes. */
> > Is size_t wide enough in the extreme case? Perhaps yes, because I
> > don't think we'll ever load 64-bit ELF on a 32-bit platform.
> 
> Even if we did, there is no chance that more than a single size_t's worth of
> data needs clearing, or the payload wouldn't fit in the current virtual
> address space.

Perhaps go even in further an add an arbitrary limit?
Like so (compile tested only):

From 22d0a6e0c6fc61a9e257ec4db78c2e58978b2976 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
Date: Tue, 6 Sep 2016 12:45:50 -0400
Subject: [PATCH] livepatch: Add limit of 2MB to payload .bss sections.

The initial patch: 11ff40fa7bb5fdcc69a58d0fec49c904ffca4793
"xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op" caps the
size of the binary at 2MB. We follow that in capping the size
of the .BSSes to be at maximum 2MB.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
 xen/common/livepatch_elf.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index 789e8fc..4a4111d 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -86,7 +86,16 @@ static int elf_resolve_sections(struct livepatch_elf *elf, 
const void *data)
                     delta < sizeof(Elf_Ehdr) ? "at ELF header" : "is past 
end");
             return -EINVAL;
         }
-
+        else if ( !(sec[i].sec->sh_flags & SHF_EXECINSTR) &&
+                  (sec[i].sec->sh_flags & SHF_WRITE) &&
+                  sec[i].sec->sh_type == SHT_NOBITS &&
+                  sec[i].sec->sh_size > MB(2) )
+        {
+            /* Arbitrary limit. */
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Section [%u] .bss is bigger 
than 2MB!\n",
+                    elf->name, i);
+            return -EINVAL;
+        }
         sec[i].data = data + delta;
         /* Name is populated in elf_resolve_section_names. */
         sec[i].name = NULL;
-- 
2.4.11


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