Xen: Fix potential page faults in the grant_table.c

[pratyush]

Summary

In gnttab_end_foreign_access_ref():
If the grant reference is invalid, doing shared[ref] could cause a page fault.

In gnttab_alloc_grant_references():
If gnttab_alloc_grant_references() fails and the code using it calls gnttab_free_grant_references(), then gnttab_entry(ref) in gnttab_free_grant_references()'s while loop will cause a page fault. But, there is a check for head == GNTTAB_LIST_END before calling it. Setting the head to GNTTAB_LIST_END when gnttab_alloc_grant_references() fails prevents the page fault.

How I discovered this:
I was attempting to run a Xen DomU in a very low grant references situation, and I got the message:
"xn0: failed to allocate tx grant refs".

Looking in the code, I discovered these two were the reason of the page fault because setup_txqs() calls disconnect_txq() when it can't connect. And disconnect_txq() calls gnttab_free_grant_references() and gnttab_end_foreign_access_ref() with invalid values, causing the page fault.

Note that netfront still panics even with these fixes, but that's a different issue, and it is out of my skill set. I will submit a detailed bug report soon.

[pratyush]

Ping.

[royger]

sys/dev/xen/grant_table/grant_table.c

187

I would turn this into a KASSERT(ref != GRANT_REF_INVALID, ("trying to free an invalid grant ref"));

I don't think calling gnttab_end_foreign_access_ref with an explicit invalid grant reference is a valid use-case.

356

I don't think you need to do this. If the function returns an error the caller shouldn't poke at head and expect it to be a valid list. I think the callers should be fixed instead of adding this workaround here.

[pratyush]

Turned the check into a KASSERT.

I will fix this problem in blkfront send the patch by tomorrow.

[pratyush]

Before you commit this, I just noticed that gnttab_free_grant_references() does

if (head == GNTTAB_LIST_END)
	return;

Maybe it should be converted to a KASSERT as well?

[pratyush]

Ping.

Thoughts on changing the condition mentioned in the previous comment to a KASSERT?

if (head == GNTTAB_LIST_END)
	return;