Subject: [PATCH v7 8/9] ARM: vdso initialization, mapping, and synchronization

From: [email-masked] (Nathan Lynch)
Date: Tue, 1 Jul 2014 09:01:14 -0500

On 06/30/2014 04:37 PM, Andy Lutomirski wrote:
> On 06/22/2014 08:11 PM, Nathan Lynch wrote:
>> +void arm_install_vdso(struct mm_struct *mm, unsigned long addr)
>> +{
>> +	int ret;
>> +
>> +	mm->context.vdso = ~0UL;
>> +
>> +	if (vdso_pagelist == NULL)
>> +		return;
>> +
>> +	/*
>> +	 * Put vDSO base into mm struct before calling
>> +	 * install_special_mapping so the perf counter mmap tracking
>> +	 * code will recognise it as a vDSO.
>> +	 */
>> +	mm->context.vdso = addr;
>> +
>> +	ret = install_special_mapping(mm, addr, vdso_mapping_len,
>> +				      VM_READ|VM_EXEC|
>> +				      VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
>> +				      vdso_pagelist);
> 
> Eek.  You're mapping the shared data VM_MAYWRITE.  This will cause
> bizarre and confusing failures if ptrace pokes at it.

I'm aware of that.  One could argue (as does the author of the
equivalent code in powerpc) that this is a "well, don't do that"
situation.  But I tend to agree that it would be nicer to prevent this
failure mode.

> You also seem to
> be sticking it *before* the vdso in the same vma.  This will severely
> piss off all the tools that assume that "[vdso]" points to an ELF object.

Hmm, which tools?  Shouldn't they be consulting AT_SYSINFO_EHDR in the
auxiliary vector instead?

> x86 calls this thing "[vvar]" and sticks it after the vdso.  You might
> not want to have the complexity of sticking it after the vdso (it's
> distinctly nontrivial), but I see nothing wrong with giving it its own
> vma just before the vdso.  The new _install_special_mapping function
> makes it considerably easier to do.

I'll give this a shot, thanks.