On Tue, Aug 14, 2012 at 09:29:49AM +0930, Rusty Russell wrote:

On Mon, 13 Aug 2012 11:41:23 +0300, "Michael S. Tsirkin" [off-list ref] wrote:

quoted

On Fri, Aug 10, 2012 at 02:55:15PM -0300, Rafael Aquini wrote:

quoted

+/*
+ * Populate balloon_mapping->a_ops->freepage method to help compaction on
+ * re-inserting an isolated page into the balloon page list.
+ */
+void virtballoon_putbackpage(struct page *page)
+{
+	spin_lock(&pages_lock);
+	list_add(&page->lru, &vb_ptr->pages);
+	spin_unlock(&pages_lock);

Could the following race trigger:
migration happens while module unloading is in progress,
module goes away between here and when the function
returns, then code for this function gets overwritten?
If yes we need locking external to module to prevent this.
Maybe add a spinlock to struct address_space?

The balloon module cannot be unloaded until it has leaked all its pages,
so I think this is safe:

        static void remove_common(struct virtio_balloon *vb)
        {
        	/* There might be pages left in the balloon: free them. */
        	while (vb->num_pages)
        		leak_balloon(vb, vb->num_pages);

Cheers,
Rusty.

I know I meant something else.
Let me lay this out:

CPU1 executes:
void virtballoon_putbackpage(struct page *page)
{
	spin_lock(&pages_lock);
	list_add(&page->lru, &vb_ptr->pages);
	spin_unlock(&pages_lock);


		at this point CPU2 unloads module:
						leak_balloon
						......

		next CPU2 loads another module so code memory gets overwritten

now CPU1 executes the next instruction:

}

which would normally return to function's caller,
but it has been overwritten by CPU2 so we get corruption.

No?

-- 
MST

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>