On Thu, Mar 28, 2013 at 11:18:21PM +0200, Mikolaj Golub wrote: > On Thu, Mar 28, 2013 at 12:51:34PM +0200, Konstantin Belousov wrote: > > > In the generic Elf 64bit draft specification I have, the notes sections > > are specified to consists of entries, each of which is an array of 8-byte > > words. I think we are right using the 8-byte alignment. > > I have impression many implementations use 4-byte alignment. E.g. in > NetBSD: > > sys/kern/core_elf32.c: > > #define ELFROUNDSIZE 4 /* XXX Should it be sizeof(Elf_Word)? */ > #define elfround(x) roundup((x), ELFROUNDSIZE) Note that this is core_elf32. I am concerned with the 64-bit cores.
> > Also, we have inconsistency with imgactl_elf.c/parse_notes(), which > uses 4-byte alignment: > > note = (const Elf_Note *)((const char *)(note + 1) + > roundup2(note->n_namesz, sizeof(Elf32_Addr)) + > roundup2(note->n_descsz, sizeof(Elf32_Addr))); > > I suppose there were no issues before, because accidentally the sizes > of all notes we had were 8 bytes aligned. Indeed, both ABI and NOINIT notes have size which is multiple of 8. > > Now, when I add new notes it will break things. I don't have strong > opinion, it will be ok for me to leave 8-byte alignment and fix > issues, just want to have strong support here :-) Well, while the issue is discussed and decided, you could just make your new notes size be multiple of 8 too. > > BTW, looking at NetBSD code I see they set p_align in the note > segement to ELFROUNDSIZE: > > /* Write out the PT_NOTE header. */ > ws.psections->p_type = PT_NOTE; > ws.psections->p_offset = notestart; > ws.psections->p_vaddr = 0; > ws.psections->p_paddr = 0; > ws.psections->p_filesz = notesize; > ws.psections->p_memsz = 0; > ws.psections->p_flags = PF_R; > ws.psections->p_align = ELFROUNDSIZE; > > while we set to 0: > > /* The note segement. */ > phdr->p_type = PT_NOTE; > phdr->p_offset = hdrsize; > phdr->p_vaddr = 0; > phdr->p_paddr = 0; > phdr->p_filesz = notesz; > phdr->p_memsz = 0; > phdr->p_flags = 0; > phdr->p_align = 0; You mean, for the core dumps ? > > Shouldn't we set it to alignment size too? Note also, they set > "Segment is readable" flag. I think both changes are fine. I skimmed over the usermode parts of the patch. One thing I did not liked is the mis-handling of the read() return values. If there is short read, the errno value is meaningless, but warn() would still append it to the message. I suggest to explicitely distinguish -1 and >= 0 returns from reads.
pgp4aWv2zWVyL.pgp
Description: PGP signature
