[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v1 1/3] xen/livepatch: Tighten alignment checks.
>>> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> 07/11/17 6:53 PM >>> >--- a/xen/common/livepatch.c >+++ b/xen/common/livepatch.c >@@ -406,6 +406,15 @@ static int move_payload(struct payload *payload, struct >livepatch_elf *elf) >ASSERT(offset[i] != UINT_MAX); > >elf->sec[i].load_addr = buf + offset[i]; >+ if ( elf->sec[i].sec->sh_addralign > 1 && I think ruling out just zero here would be sufficient and look more "natural". >+ ((Elf_Addr)elf->sec[i].load_addr % >elf->sec[i].sec->sh_addralign) ) The cast here mkes me wonder what it is that you're checking, or whether the check isn't being done later than it should be done: I'd expect all such to happen on the original section header, where no pointer types exist (and hence such a cast shouldn't be needed). And then - what about the alignment not being a power of 2? Is that well defined? >--- a/xen/common/livepatch_elf.c >+++ b/xen/common/livepatch_elf.c >@@ -86,6 +86,13 @@ 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 && >+ sec[i].sec->sh_addr % sec[i].sec->sh_addralign ) Ah, here it is - why the second check further up then? >+ { >+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Section [%u] size >(%"PRIuElfWord") is not aligned properly (%"PRIuElfWord")\n", >+ elf->name, i, sec[i].sec->sh_size, >sec[i].sec->sh_addralign); What does section size (being logged here) have to do with the check that failed? I also question the use of decimal values here - generally I find sizes, offsets, and alignments easier to deal with when they are being presented as hex numbers. >--- a/xen/include/xen/elfstructs.h >+++ b/xen/include/xen/elfstructs.h >@@ -555,6 +555,7 @@ typedef struct { > >#if defined(ELFSIZE) && (ELFSIZE == 32) >#define PRIxElfAddr "08x" >+#define PRIuElfWord "08u" And leading zeros would cause even more confusion, as they would suggest (at least to C programmers) the number to be octal. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |