Re: [xen-unstable test] 123379: regressions - FAIL

From: Juergen Gross <jgross@suse.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Wei Liu <wei.liu2@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [xen-unstable test] 123379: regressions - FAIL
Date: Wed, 13 Jun 2018 09:57:39 +0200	[thread overview]
Message-ID: <383a47e7-790b-21af-dd59-f454c9df1e75@suse.com> (raw)
In-Reply-To: <5B20C5E002000078001CAB80@suse.com>

On 13/06/18 09:21, Jan Beulich wrote:
>>>> On 13.06.18 at 08:50, <jgross@suse.com> wrote:
>> On 13/06/18 08:11, Jan Beulich wrote:
>>> Teaching the privcmd driver of all
>>> the indirections in hypercall request structures doesn't look very
>>> attractive (or maintainable). Or are you thinking of the caller
>>> providing sideband information describing the buffers involved,
>>> perhaps along the lines of how dm_op was designed?
>>
>> I thought about that, yes. libxencall already has all the needed data
>> for that. Another possibility would be a dedicated ioctl for registering
>> a hypercall buffer (or some of them).
> 
> I'm not sure that's an option: Is it legitimate (secure) to retain the
> effects of get_user_pages() across system calls?

I have to check that.

>>> There's another option, but that has potentially severe drawbacks
>>> too: Instead of returning -EFAULT on buffer access issues, we
>>> could raise #PF on the very hypercall insn. Maybe something to
>>> consider as an opt-in for PV/PVH, and as default for HVM.
>>
>> Hmm, I'm not sure this will solve any problem. I'm not aware that it
>> is considered good practice to just access a user buffer from kernel
>> without using copyin()/copyout() when you haven't locked the page(s)
>> via get_user_pages(), even if the buffer was mlock()ed. Returning
>> -EFAULT is the right thing to do, I believe.
> 
> But we're talking about the very copyin()/copyout(), just that here
> it's being amortized by doing the operation just once (in the
> hypervisor). A #PF would arise from syscall buffer copyin()/copyout(),
> and the suggestion was to produce the same effect for the squashed
> operation. Perhaps we wouldn't want #PF to come back from ordinary
> (kernel invoked) hypercalls, but ones relayed by privcmd are different
> in many ways anyway (see the stac()/clac() pair around the actual
> call, for example).

Aah, okay. This is an option, but it would require some kind of
interface to tell the hypervisor it should raise the #PF instead of
returning -EFAULT, of course, as the kernel has to be prepared for
that.

I like that idea very much!

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel