Re: [Xen-devel] [PATCH v2 1/5] livepatch: Tighten alignment checks.

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Konrad Rzeszutek Wilk <konrad@xxxxxxxxxx> 07/26/17 9:50 PM >>>
>--- 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 > 1 &&

As said before, to me this check looks confusing. I'd recommend to only check
for the field to be non-zero.

>+                  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 > 1 && sec[i].sec->sh_addralign % 
>2 )

What use is this one? Do you perhaps mean to check that the alignment is
a power of 2? In that case a single check of sh_addralign & (sh_addralign - 1)
against zero would be what you want.

Jan


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