On Sat, May 30, 2020 at 08:42:32PM +0100, Al Viro wrote:
> On Sat, May 30, 2020 at 12:20:54PM -0700, Linus Torvalds wrote:
> > On Sat, May 30, 2020 at 12:14 PM Al Viro wrote:
> > >
> > > > And none of that code verifies that the end result is a user address.
> > >
> > > kvm_is_error_hva() is
> > >
On Sat, May 30, 2020 at 12:20:54PM -0700, Linus Torvalds wrote:
> On Sat, May 30, 2020 at 12:14 PM Al Viro wrote:
> >
> > > And none of that code verifies that the end result is a user address.
> >
> > kvm_is_error_hva() is
> > return addr >= PAGE_OFFSET;
>
> Ahh, that's what I missed. It
On Sat, May 30, 2020 at 08:19:40PM +0100, Al Viro wrote:
> On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
>
> > And I don't understand why you mention set_fs() vs access_ok(). None
> > of this code has anything that messes with set_fs(). The access_ok()
> > is garbage and shouldn'
On Sat, May 30, 2020 at 12:14 PM Al Viro wrote:
>
> > And none of that code verifies that the end result is a user address.
>
> kvm_is_error_hva() is
> return addr >= PAGE_OFFSET;
Ahh, that's what I missed. It won't work on other architectures, but
within x86 it's fine.
On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
> And I don't understand why you mention set_fs() vs access_ok(). None
> of this code has anything that messes with set_fs(). The access_ok()
> is garbage and shouldn't exist, and those user accesses should all use
> the checking vers
On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
>
> It really isn't.
>
> Your very first statement shows how broken it is:
>
> > FWIW, the kvm side of things (vhost is yet another pile of fun) is
> >
> > [x86] kvm_hv_set_msr_pw():
> > arch/x86/kvm/hyperv.c:1027: if (_
On Sat, May 30, 2020 at 11:39 AM Al Viro wrote:
>
> Actually, it's somewhat less brittle than you think (on non-mips, at least)
> and not due to those long-ago access_ok().
It really isn't.
Your very first statement shows how broken it is:
> FWIW, the kvm side of things (vhost is yet another pi
On Sat, May 30, 2020 at 10:57:24AM -0700, Linus Torvalds wrote:
> So no. I disagree. There is absolutely nothing "obviously ok" about
> any of that kvm code. Quite the reverse.
>
> I'd argue that it's very much obviously *NOT* ok, even while it might
> just happen to work.
Actually, it's somewha
On Sat, May 30, 2020 at 9:20 AM Paolo Bonzini wrote:
>
> Yes, the access_ok is done in __kvm_set_memory_region and gfn_to_hva()
> returns a page-aligned address so it's obviously ok for a u32.
It's not that it's "obviously ok for an u32".
It is _not_ obviously ok for a user address. There's actu
> On Fri, May 29, 2020 at 04:52:59PM -0700, Linus Torvalds wrote:
>> It looks like the argument for the address being validated is that it
>> comes from "gfn_to_hva()", which should only return
>> host-virtual-addresses. That may be true.
Yes, the access_ok is done in __kvm_set_memory_region and g
On Sat, May 30, 2020 at 03:31:47PM +0100, Al Viro wrote:
> It's a bit trickier than that, but I want to deal with that at the same
> time as the rest of kvm/vhost stuff. So for this series I just went
> for minimal change. There's quite a pile of vhost and kvm stuff,
> but it's not ready yet - w
On Fri, May 29, 2020 at 04:52:59PM -0700, Linus Torvalds wrote:
> On Fri, May 29, 2020 at 4:27 PM Al Viro wrote:
> > a/arch/x86/kvm/hyperv.c
> > - if (__clear_user((void __user *)addr, sizeof(u32)))
> > + if (__put_user(0, (u32 __user *)addr))
>
> I'm not doubting that
On Fri, May 29, 2020 at 4:27 PM Al Viro wrote:
> a/arch/x86/kvm/hyperv.c
> - if (__clear_user((void __user *)addr, sizeof(u32)))
> + if (__put_user(0, (u32 __user *)addr))
I'm not doubting that this is a correct transformation and an
improvement, but why is it using th
From: Al Viro
Signed-off-by: Al Viro
---
arch/x86/kvm/hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..b85b211d4676 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1129,7 +1129,7 @@
14 matches
Mail list logo
