Re: [x86] copy_from{to}_user question

[yalin wang <yalin.wang2010@gmail.com>]

> On Aug 12, 2015, at 18:07, Borislav Petkov <bp@suse.de> wrote:
> 
> On Wed, Aug 12, 2015 at 05:01:14PM +0800, yalin wang wrote:
>> hi x86 maintainers,
>> 
>> i have a question about copy_from{to}_user() function,
>> i find on other platforms like arm/ arm64 /hexagon,
>> all copy_from{to}_user function only check source address for
>> copy_from and only check to address for copy_to user function,
>> never check both source and dest together,
>> 
>> but on x86 platform, i see copy_from{to}_user use a generic function
>> named copy_user_generic_unrolled() in arch/x86/lib/copy_user_64.S,
> 
> That one is the fallback and used only on machines which don't set
> X86_FEATURE_REP_GOOD or X86_FEATURE_ERMS. Basically old P4 and K7 and
> early K8s.
> 
i see, generically, it use 3 function for different processors,

static __always_inline __must_check unsigned long
copy_user_generic(void *to, const void *from, unsigned len)
{
	unsigned ret;

	/*
	 * If CPU has ERMS feature, use copy_user_enhanced_fast_string.
	 * Otherwise, if CPU has rep_good feature, use copy_user_generic_string.
	 * Otherwise, use copy_user_generic_unrolled.
	 */
	alternative_call_2(copy_user_generic_unrolled,
			 copy_user_generic_string,
			 X86_FEATURE_REP_GOOD,
			 copy_user_enhanced_fast_string,
			 X86_FEATURE_ERMS,
			 ASM_OUTPUT2("=a" (ret), "=D" (to), "=S" (from),
				     "=d" (len)),
			 "1" (to), "2" (from), "3" (len)
			 : "memory", "rcx", "r8", "r9", "r10", "r11");
	return ret;
}

>> it check source and dest address no matter it is copy_from user or
>> copy_to_user ,  is it correct? 
>> for copy_from_user i think only need check source address is enough,
> 
> How else would we be able to use the same function in copy_to and
> copy_from variants?

for 3 methods implemented here, i think can implemented by add one more function parameter,
like this:
#define COPY_FROM 0
#define COPY_TO 1
#define COPY_IN 2
copy_user_generic(void *to, const void *from, unsigned len, int type)

we store type into one fix register, for example r12 ,
then in fix up code, we can know the exception is caused by copy_from
copy_to or copy_in user function by check r12 value(0 , 1 ,2 value), then if 
it is copy_from, we only allow read fault, if the exception is write fault, panic() .

the same rules also apply to copy_to / copy_in function .

is it possible to change it like this ?

Thanks