diff mbox

[v3,02/17] livepatch: Tighten alignment checks.

Message ID 20170912003726.368-3-konrad.wilk@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Konrad Rzeszutek Wilk Sept. 12, 2017, 12:37 a.m. UTC 
The ELF specification mentions nothing about the sh_size being
modulo the sh_addralign. Only that sh_addr MUST be aligned on
sh_addralign if sh_addralign is not zero or one.

We on loading did not take this in-to account so this patch adds
a check on the ELF file as it is being parsed.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
v1: Initial patch
v2: Drop the check when loading it in memory
    Add check for alignment being anything but power of two (ignoring 0, and 1)
    Change dprintk to include hex values and print addr not size.
v3: Change the two checks to be per Jan's recommendations.
---
 xen/common/livepatch_elf.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Jan Beulich Sept. 12, 2017, 2:28 p.m. UTC | #1 
>>> On 12.09.17 at 02:37, <konrad@kernel.org> wrote:
> The ELF specification mentions nothing about the sh_size being
> modulo the sh_addralign. Only that sh_addr MUST be aligned on
> sh_addralign if sh_addralign is not zero or one.
> 
> We on loading did not take this in-to account so this patch adds
> a check on the ELF file as it is being parsed.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit generally I'd recommend the check to be done in the
opposite order.

Jan
diff mbox

Patch

diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index b69e2718dd..7839913ff5 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -86,6 +86,19 @@  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_addralign &&
+                  sec[i].sec->sh_addr % sec[i].sec->sh_addralign )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Section [%u] addr (%#"PRIxElfAddr") is not aligned properly (%#"PRIxElfAddr")\n",
+                    elf->name, i, sec[i].sec->sh_addr, sec[i].sec->sh_addralign);
+            return -EINVAL;
+        }
+        else if ( sec[i].sec->sh_addralign & (sec[i].sec->sh_addralign - 1) )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Section [%u] alignment (%#"PRIxElfAddr") is not supported\n",
+                    elf->name, i, sec[i].sec->sh_addralign);
+            return -EOPNOTSUPP;
+        }
         else if ( (sec[i].sec->sh_flags & (SHF_WRITE | SHF_ALLOC)) &&
                   sec[i].sec->sh_type == SHT_NOBITS &&
                   sec[i].sec->sh_size > LIVEPATCH_MAX_SIZE )