Skip to contentSkip to conversation

gnttab_setup_table error case

PATCH In review Diego Ongaro 3 participants 4 messages Jul 30, 2008 – Jul 31, 2008

lore
Share & tools
lore

Diego Ongaro

author
#0 lore raw

PATCH 1/1 gnttab_setup_table error case

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 <[email protected]>

---

xen/common/grant_table.c +7 -0
@@ -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:
#1 lore raw
Quoted 6 lines · Diego Ongaro

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

Quoted 27 lines
Signed-off-by Diego Ongaro <[email protected]>

---

xen/common/grant_table.c +7 -0
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw
Quoted 9 lines · Qing He, le Thu 31 Jul 2008 10:08:28 +0800, a écrit :
Quoted 6 lines · Diego Ongaro

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.

Quoted 2 lines

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 :)

Quoted 1 line

1. let setup_table return -ENOSYS again in HVM?

That could make sense.

Samuel

author
#3 lore raw
Quoted 11 lines · Samuel Thibault
Quoted 8 lines · Qing He, le Thu 31 Jul 2008 10:08:28 +0800, a écrit :
Quoted 6 lines · Diego Ongaro

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 :)

Quoted 6 lines
Quoted 2 lines

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