Commit 72cb9ee4 authored by Yufan Chen's avatar Yufan Chen Committed by Dominique Martinet
Browse files

9p/trans_xen: make cleanup idempotent after dataring alloc errors 



xen_9pfs_front_alloc_dataring() tears down resources on failure but
leaves ring fields stale. If xen_9pfs_front_init() later jumps to the
common error path, xen_9pfs_front_free() may touch the same resources
again, causing duplicate/invalid gnttab_end_foreign_access() calls and
potentially dereferencing a freed intf pointer.

Initialize dataring sentinels before allocation, gate teardown on those
sentinels, and clear ref/intf/data/irq immediately after each release.

This keeps cleanup idempotent for partially initialized rings and
prevents repeated teardown during init failure handling.

Signed-off-by: default avatarYufan Chen <ericterminal@gmail.com>
Reviewed-by: default avatarStefano Stabellini <sstabellini@kernel.org>
Message-ID: <20260324153023.86853-2-ericterminal@gmail.com>
Signed-off-by: default avatarDominique Martinet <asmadeus@codewreck.org>
parent 890d5696

net/9p/trans_xen.c

+37 −14
Original line number Diff line number Diff line
@@ -283,25 +283,33 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv) 


			cancel_work_sync(&ring->work);



			if (!priv->rings[i].intf)

			if (!ring->intf)

				break;

			if (priv->rings[i].irq > 0)

				unbind_from_irqhandler(priv->rings[i].irq, ring);

			if (priv->rings[i].data.in) {

				for (j = 0;

				     j < (1 << priv->rings[i].intf->ring_order);

			if (ring->irq >= 0) {

				unbind_from_irqhandler(ring->irq, ring);

				ring->irq = -1;

			}

			if (ring->data.in) {

				for (j = 0; j < (1 << ring->intf->ring_order);

				     j++) {

					grant_ref_t ref;



					ref = priv->rings[i].intf->ref[j];

					ref = ring->intf->ref[j];

					gnttab_end_foreign_access(ref, NULL);

					ring->intf->ref[j] = INVALID_GRANT_REF;

				}

				free_pages_exact(priv->rings[i].data.in,

				   1UL << (priv->rings[i].intf->ring_order +

				free_pages_exact(ring->data.in,

						 1UL << (ring->intf->ring_order +

							 XEN_PAGE_SHIFT));

				ring->data.in = NULL;

				ring->data.out = NULL;

			}

			if (ring->ref != INVALID_GRANT_REF) {

				gnttab_end_foreign_access(ring->ref, NULL);

				ring->ref = INVALID_GRANT_REF;

			}

			gnttab_end_foreign_access(priv->rings[i].ref, NULL);

			free_page((unsigned long)priv->rings[i].intf);

			free_page((unsigned long)ring->intf);

			ring->intf = NULL;

		}

		kfree(priv->rings);

	}
@@ -334,6 +342,12 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev, 
	int ret = -ENOMEM;

	void *bytes = NULL;



	ring->intf = NULL;

	ring->data.in = NULL;

	ring->data.out = NULL;

	ring->ref = INVALID_GRANT_REF;

	ring->irq = -1;



	init_waitqueue_head(&ring->wq);

	spin_lock_init(&ring->lock);

	INIT_WORK(&ring->work, p9_xen_response);
@@ -379,9 +393,18 @@ static int xen_9pfs_front_alloc_dataring(struct xenbus_device *dev, 
		for (i--; i >= 0; i--)

			gnttab_end_foreign_access(ring->intf->ref[i], NULL);

		free_pages_exact(bytes, 1UL << (order + XEN_PAGE_SHIFT));

		ring->data.in = NULL;

		ring->data.out = NULL;

	}

	if (ring->ref != INVALID_GRANT_REF) {

		gnttab_end_foreign_access(ring->ref, NULL);

		ring->ref = INVALID_GRANT_REF;

	}

	if (ring->intf) {

		free_page((unsigned long)ring->intf);

		ring->intf = NULL;

	}

	ring->irq = -1;

	return ret;

}