Re: arch/x86/kernel/cpu/sgx/virt.c:295:36: sparse: sparse: cast removes address space '__user' of expression

[David Laight <David.Laight@ACULAB.COM>]

[-- Attachment #1: Type: text/plain, Size: 2370 bytes --]

From: kernel test robot
> Sent: 22 February 2022 00:43
...
> vim +/__user +295 arch/x86/kernel/cpu/sgx/virt.c
> 
>    260
>    261	/**
>    262	 * sgx_virt_ecreate() - Run ECREATE on behalf of guest
>    263	 * @pageinfo:	Pointer to PAGEINFO structure
>    264	 * @secs:	Userspace pointer to SECS page
>    265	 * @trapnr:	trap number injected to guest in case of ECREATE error
>    266	 *
>    267	 * Run ECREATE on behalf of guest after KVM traps ECREATE for the purpose
>    268	 * of enforcing policies of guest's enclaves, and return the trap number
>    269	 * which should be injected to guest in case of any ECREATE error.
>    270	 *
>    271	 * Return:
>    272	 * -  0:	ECREATE was successful.
>    273	 * - <0:	on error.
>    274	 */
>    275	int sgx_virt_ecreate(struct sgx_pageinfo *pageinfo, void __user *secs,
>    276			     int *trapnr)
>    277	{
>    278		int ret;
>    279
>    280		/*
>    281		 * @secs is an untrusted, userspace-provided address.  It comes from
>    282		 * KVM and is assumed to be a valid pointer which points somewhere in
>    283		 * userspace.  This can fault and call SGX or other fault handlers when
>    284		 * userspace mapping @secs doesn't exist.
>    285		 *
>    286		 * Add a WARN() to make sure @secs is already valid userspace pointer
>    287		 * from caller (KVM), who should already have handled invalid pointer
>    288		 * case (for instance, made by malicious guest).  All other checks,
>    289		 * such as alignment of @secs, are deferred to ENCLS itself.
>    290		 */
>    291		if (WARN_ON_ONCE(!access_ok(secs, PAGE_SIZE)))
>    292			return -EINVAL;
>    293
>    294		__uaccess_begin();
>  > 295		ret = __ecreate(pageinfo, (void *)secs);
>    296		__uaccess_end();

How on earth is that expected to work - or am I missing something.

If accessing 'secs' page faults then doesn't the address on the
instruction that faults need to be in the exception table (or
whatever it is called) in order for the kernel not to panic?

Isn't that even true if pagefault_disable() is called to stop
the page being loaded?

The only way the above can work is if the relevant user pages
are 'locked' so they can't possibly fault.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)