gnttab_setup_table should set an error status code if the gmfn it gets
for a grant table page is invalid.
I ran into this issue when I tried to set up the grant table during hvm
domain creation, and it caused a BUG_ON later down the line. With this
patch, the hypercall will gracefully fail instead.
Signed-off-by: Diego Ongaro <diego.ongaro@citrix.com>
---
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -913,8 +913,15 @@ gnttab_setup_table(
op.status = GNTST_okay;
for ( i = 0; i < op.nr_frames; i++ )
{
gmfn = gnttab_shared_gmfn(d, d->grant_table, i);
+ if ( gmfn == -1 )
+ {
+ gdprintk(XENLOG_INFO,
+ "grant table gmfn unavailable\n");
+ op.status = GNTST_general_error;
+ goto out3;
+ }
(void)copy_to_guest_offset(op.frame_list, i, &gmfn, 1);
}
out3:
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
On Wed, 2008-07-30 at 14:17 +0100, Diego Ongaro wrote:> gnttab_setup_table should set an error status code if the gmfn it gets > for a grant table page is invalid. > > I ran into this issue when I tried to set up the grant table during hvm > domain creation, and it caused a BUG_ON later down the line. With this > patch, the hypercall will gracefully fail instead.Do you use 32bit guest on top of 64bit hypervisor? In that case op.frame_list has different size in the hypervisor and the guest, and the BUG_ON is probably triggered because of the hypervisor ruining the stack. Just curious, but why we want to call setup_table in HVM in the first place. Since HVM has its isolated address space, it will always fail. It seems that setup_table is added to HVM because of this: http://lists.xensource.com/archives/html/xen-devel/2008-03/msg00927.html At that time, calling setup_table is a guest bug and I think it''s already fixed by removing that call. So, can we: 1. let setup_table return -ENOSYS again in HVM? or 2. call something like compat_grant_table_op instead in 32/64? or 2. add a stub in hvm_grant_table_op so that it doesn''t bother calling {do,compat}_grant_table_op at all? Thanks, Qing> > Signed-off-by: Diego Ongaro <diego.ongaro@citrix.com> > --- > diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c > --- a/xen/common/grant_table.c > +++ b/xen/common/grant_table.c > @@ -913,8 +913,15 @@ gnttab_setup_table( > op.status = GNTST_okay; > for ( i = 0; i < op.nr_frames; i++ ) > { > gmfn = gnttab_shared_gmfn(d, d->grant_table, i); > + if ( gmfn == -1 ) > + { > + gdprintk(XENLOG_INFO, > + "grant table gmfn unavailable\n"); > + op.status = GNTST_general_error; > + goto out3; > + } > (void)copy_to_guest_offset(op.frame_list, i, &gmfn, 1); > } > > out3: > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault
2008-Jul-31 08:58 UTC
Re: [Xen-devel] [PATCH] gnttab_setup_table error case
Qing He, le Thu 31 Jul 2008 10:08:28 +0800, a écrit :> On Wed, 2008-07-30 at 14:17 +0100, Diego Ongaro wrote: > > gnttab_setup_table should set an error status code if the gmfn it gets > > for a grant table page is invalid. > > > > I ran into this issue when I tried to set up the grant table during hvm > > domain creation, and it caused a BUG_ON later down the line. With this > > patch, the hypercall will gracefully fail instead. > > Do you use 32bit guest on top of 64bit hypervisor?No, just 64/64.> Just curious, but why we want to call setup_table in HVM in the first > place. Since HVM has its isolated address space, it will always fail.That was for his personal project. The call was made from dom0. The problem is that it triggers a BUG_ON, which we do not really want to happen :)> 1. let setup_table return -ENOSYS again in HVM?That could make sense. Samuel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Samuel Thibault wrote:> Qing He, le Thu 31 Jul 2008 10:08:28 +0800, a écrit : >> On Wed, 2008-07-30 at 14:17 +0100, Diego Ongaro wrote: >>> gnttab_setup_table should set an error status code if the gmfn it gets >>> for a grant table page is invalid. >>> >>> I ran into this issue when I tried to set up the grant table during hvm >>> domain creation, and it caused a BUG_ON later down the line. With this >>> patch, the hypercall will gracefully fail instead. >> Do you use 32bit guest on top of 64bit hypervisor? > > No, just 64/64.Well, no, it was 32 on 64 :)>> Just curious, but why we want to call setup_table in HVM in the first >> place. Since HVM has its isolated address space, it will always fail. > > That was for his personal project. The call was made from dom0. The > problem is that it triggers a BUG_ON, which we do not really want to > happen :)Keir''s xen-unstable.hg cs 18177:9ee2e41a68a1 simply removed the problematic BUG_ON, so at least userspace can now handle the error. -Diego _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel