[CRIU] [PATCH 4/8] vdso: share the PIE part of the vDSO proxy machinery between all architectures

[Christopher Covington]

On 03/10/2014 04:51 AM, Alexander Kartashov wrote:
> This patch splits the file arch/x86/vdso-pie.c into machine-dependent
> and machine-independent parts by moving the routines vdso_fill_symtable(),
> vdso_remap(), and vdso_proxify() to the file pie/vdso.c.
> 
> The ARM version of the routines is moved to the source pie/vdso-stub.c
> to provide the vDSO proxy stub implementation for architectures
> that don't provide the vDSO.

> diff --git a/pie/vdso.c b/pie/vdso.c
> new file mode 100644
> index 0000000..1dcda48
> --- /dev/null
> +++ b/pie/vdso.c

> +int vdso_fill_symtable(char *mem, size_t size, struct vdso_symtable *t)
> +{
> +	Elf64_Ehdr *ehdr = (void *)mem;
> +	Elf64_Shdr *shdr, *shdr_strtab;
> +	Elf64_Shdr *shdr_dynsym;
> +	Elf64_Shdr *shdr_dynstr;
> +	Elf64_Phdr *phdr;
> +	Elf64_Shdr *text;
> +	Elf64_Sym *sym;

This doesn't appear to be generic across all ABI's. Maybe the file should be
called vdso64 or something like that?

> +
> +	char *section_names, *dynsymbol_names;
> +
> +	unsigned long base = VDSO_BAD_ADDR;
> +	unsigned int i, j, k;
> +
> +	/*
> +	 * Elf header bytes. For detailed
> +	 * description see Elf specification.
> +	 */
> +	char vdso_ident[] = {
> +		0x7f, 0x45, 0x4c, 0x46, 0x02, 0x01, 0x01, 0x00,
> +		0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> +	};
> +
> +	char *vdso_x86_symbols[VDSO_SYMBOL_MAX] = {
> +		[VDSO_SYMBOL_GETTIMEOFDAY]	= VDSO_SYMBOL_GETTIMEOFDAY_NAME,
> +		[VDSO_SYMBOL_GETCPU]		= VDSO_SYMBOL_GETCPU_NAME,
> +		[VDSO_SYMBOL_CLOCK_GETTIME]	= VDSO_SYMBOL_CLOCK_GETTIME_NAME,
> +		[VDSO_SYMBOL_TIME]		= VDSO_SYMBOL_TIME_NAME,
> +	};

Should the above snippet stay in a machine-specific location?

> +	BUILD_BUG_ON(sizeof(vdso_ident) != sizeof(ehdr->e_ident));
> +
> +	pr_debug("Parsing at %lx %lx\n",
> +		 (long)mem, (long)mem + (long)size);
> +
> +	/*
> +	 * Make sure it's a file we support.
> +	 */
> +	if (builtin_memcmp(ehdr->e_ident, vdso_ident, sizeof(vdso_ident))) {
> +		pr_debug("Elf header magic mismatch\n");
> +		goto err;
> +	}
> +
> +	/*
> +	 * Figure out base virtual address.
> +	 */
> +	phdr = (void *)&mem[ehdr->e_phoff];
> +	for (i = 0; i < ehdr->e_phnum; i++, phdr++) {
> +		if (__ptr_oob(phdr, mem, size))
> +			goto err;
> +		if (phdr->p_type == PT_LOAD) {
> +			base = phdr->p_vaddr;
> +			break;
> +		}
> +	}
> +	if (base != VDSO_BAD_ADDR) {
> +		pr_debug("Base address %lx\n", base);
> +	} else {
> +		pr_debug("No base address found\n");
> +		goto err;
> +	}
> +
> +	/*
> +	 * Where the section names lays.
> +	 */
> +	if (ehdr->e_shstrndx == SHN_UNDEF) {
> +		pr_err("Section names are not found\n");
> +		goto err;
> +	}
> +
> +	shdr = (void *)&mem[ehdr->e_shoff];
> +	shdr_strtab = &shdr[ehdr->e_shstrndx];
> +	if (__ptr_oob(shdr_strtab, mem, size))
> +	{
> +		pr_err("shdr_strtab is outside the vDSO\n");
> +		goto err;
> +	}
> +
> +	section_names = (void *)&mem[shdr_strtab->sh_offset];
> +	shdr_dynsym = shdr_dynstr = text = NULL;
> +
> +	for (i = 0; i < ehdr->e_shnum; i++, shdr++) {
> +		if (__ptr_oob(shdr, mem, size))
> +			goto err;
> +		if (__ptr_oob(&section_names[shdr->sh_name], mem, size))
> +			goto err;
> +
> +#if 0
> +		pr_debug("section: %2d -> %s\n",
> +			 i, &section_names[shdr->sh_name]);
> +#endif

My original instinct was to say that this needs fixing but I see it was copied
over.

> +		if (shdr->sh_type == SHT_DYNSYM &&
> +		    builtin_strcmp(&section_names[shdr->sh_name],
> +				   ".dynsym") == 0) {
> +			shdr_dynsym = shdr;
> +		} else if (shdr->sh_type == SHT_STRTAB &&
> +			   builtin_strcmp(&section_names[shdr->sh_name],
> +					  ".dynstr") == 0) {
> +			shdr_dynstr = shdr;
> +		} else if (shdr->sh_type == SHT_PROGBITS &&
> +			   builtin_strcmp(&section_names[shdr->sh_name],
> +					  ".text") == 0) {
> +			text = shdr;
> +		}
> +	}
> +
> +	if (!shdr_dynsym || !shdr_dynstr || !text) {
> +		pr_debug("No required sections found\n");
> +		goto err;
> +	}
> +
> +	dynsymbol_names = (void *)&mem[shdr_dynstr->sh_offset];
> +	if (__ptr_oob(dynsymbol_names, mem, size)	||
> +	    __ptr_oob(shdr_dynsym, mem, size)		||
> +	    __ptr_oob(text, mem, size))
> +		goto err;
> +
> +	/*
> +	 * Walk over global symbols and choose ones we need.
> +	 */
> +	j = shdr_dynsym->sh_size / sizeof(*sym);
> +	sym = (void *)&mem[shdr_dynsym->sh_offset];
> +
> +	for (i = 0; i < j; i++, sym++) {
> +		if (__ptr_oob(sym, mem, size))
> +			goto err;
> +
> +		if (ELF64_ST_BIND(sym->st_info) != STB_GLOBAL ||
> +		    ELF64_ST_TYPE(sym->st_info) != STT_FUNC)
> +			continue;
> +
> +		if (__ptr_oob(&dynsymbol_names[sym->st_name], mem, size))
> +			goto err;
> +
> +		k = get_symbol_index(&dynsymbol_names[sym->st_name],
> +				     vdso_x86_symbols,
> +				     ARRAY_SIZE(vdso_x86_symbols));
> +		if (k != VDSO_SYMBOL_MAX) {
> +			builtin_memcpy(t->symbols[k].name, vdso_x86_symbols[k],
> +				       sizeof(t->symbols[k].name));
> +			t->symbols[k].offset = (unsigned long)sym->st_value - base;

I think this will need to become generic.

> +#if 0
> +			pr_debug("symbol: %#-16lx %2d %s\n",
> +				 t->symbols[k].offset, sym->st_shndx, t->symbols[k].name);
> +#endif

My original instinct was to say that this needs fixing but I see it was copied
over.

Regards,
Christopher

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.