Skip to content

Commit ce8ded2

Browse files
sstabellinimartinetd
sstabellini
authored and
martinetd
committed
9p/xen: protect xen_9pfs_front_free against concurrent calls
The xenwatch thread can race with other back-end change notifications and call xen_9pfs_front_free() twice, hitting the observed general protection fault due to a double-free. Guard the teardown path so only one caller can release the front-end state at a time, preventing the crash. This is a fix for the following double-free: [ 27.052347] Oops: general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] SMP DEBUG_PAGEALLOC NOPTI [ 27.052357] CPU: 0 UID: 0 PID: 32 Comm: xenwatch Not tainted 6.18.0-02087-g51ab33fc0a8b-dirty #60 PREEMPT(none) [ 27.052363] RIP: e030:xen_9pfs_front_free+0x1d/0x150 [ 27.052368] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 41 55 41 54 55 48 89 fd 48 c7 c7 48 d0 92 85 53 e8 cb cb 05 00 48 8b 45 08 48 8b 55 00 <48> 3b 28 0f 85 f9 28 35 fe 48 3b 6a 08 0f 85 ef 28 35 fe 48 89 42 [ 27.052377] RSP: e02b:ffffc9004016fdd0 EFLAGS: 00010246 [ 27.052381] RAX: 6b6b6b6b6b6b6b6b RBX: ffff88800d66e400 RCX: 0000000000000000 [ 27.052385] RDX: 6b6b6b6b6b6b6b6b RSI: 0000000000000000 RDI: 0000000000000000 [ 27.052389] RBP: ffff88800a887040 R08: 0000000000000000 R09: 0000000000000000 [ 27.052393] R10: 0000000000000000 R11: 0000000000000000 R12: ffff888009e46b68 [ 27.052397] R13: 0000000000000200 R14: 0000000000000000 R15: ffff88800a887040 [ 27.052404] FS: 0000000000000000(0000) GS:ffff88808ca57000(0000) knlGS:0000000000000000 [ 27.052408] CS: e030 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 27.052412] CR2: 00007f9714004360 CR3: 0000000004834000 CR4: 0000000000050660 [ 27.052418] Call Trace: [ 27.052420] <TASK> [ 27.052422] xen_9pfs_front_changed+0x5d5/0x720 [ 27.052426] ? xenbus_otherend_changed+0x72/0x140 [ 27.052430] ? __pfx_xenwatch_thread+0x10/0x10 [ 27.052434] xenwatch_thread+0x94/0x1c0 [ 27.052438] ? __pfx_autoremove_wake_function+0x10/0x10 [ 27.052442] kthread+0xf8/0x240 [ 27.052445] ? __pfx_kthread+0x10/0x10 [ 27.052449] ? __pfx_kthread+0x10/0x10 [ 27.052452] ret_from_fork+0x16b/0x1a0 [ 27.052456] ? __pfx_kthread+0x10/0x10 [ 27.052459] ret_from_fork_asm+0x1a/0x30 [ 27.052463] </TASK> [ 27.052465] Modules linked in: [ 27.052471] ---[ end trace 0000000000000000 ]--- Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> Message-ID: <20260129230348.2390470-1-stefano.stabellini@amd.com> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
1 parent 51ffeab commit ce8ded2

File tree

1 file changed

+44
-41
lines changed

1 file changed

+44
-41
lines changed

‎net/9p/trans_xen.c‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -277,45 +277,52 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
277277
{
278278
int i, j;
279279

280-
write_lock(&xen_9pfs_lock);
281-
list_del(&priv->list);
282-
write_unlock(&xen_9pfs_lock);
283-
284-
for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
285-
struct xen_9pfs_dataring *ring = &priv->rings[i];
286-
287-
cancel_work_sync(&ring->work);
288-
289-
if (!priv->rings[i].intf)
290-
break;
291-
if (priv->rings[i].irq > 0)
292-
unbind_from_irqhandler(priv->rings[i].irq, ring);
293-
if (priv->rings[i].data.in) {
294-
for (j = 0;
295-
j < (1 << priv->rings[i].intf->ring_order);
296-
j++) {
297-
grant_ref_t ref;
298-
299-
ref = priv->rings[i].intf->ref[j];
300-
gnttab_end_foreign_access(ref, NULL);
301-
}
302-
free_pages_exact(priv->rings[i].data.in,
280+
if (priv->rings) {
281+
for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
282+
struct xen_9pfs_dataring *ring = &priv->rings[i];
283+
284+
cancel_work_sync(&ring->work);
285+
286+
if (!priv->rings[i].intf)
287+
break;
288+
if (priv->rings[i].irq > 0)
289+
unbind_from_irqhandler(priv->rings[i].irq, ring);
290+
if (priv->rings[i].data.in) {
291+
for (j = 0;
292+
j < (1 << priv->rings[i].intf->ring_order);
293+
j++) {
294+
grant_ref_t ref;
295+
296+
ref = priv->rings[i].intf->ref[j];
297+
gnttab_end_foreign_access(ref, NULL);
298+
}
299+
free_pages_exact(priv->rings[i].data.in,
303300
1UL << (priv->rings[i].intf->ring_order +
304301
XEN_PAGE_SHIFT));
302+
}
303+
gnttab_end_foreign_access(priv->rings[i].ref, NULL);
304+
free_page((unsigned long)priv->rings[i].intf);
305305
}
306-
gnttab_end_foreign_access(priv->rings[i].ref, NULL);
307-
free_page((unsigned long)priv->rings[i].intf);
306+
kfree(priv->rings);
308307
}
309-
kfree(priv->rings);
310308
kfree(priv->tag);
311309
kfree(priv);
312310
}
313311

314312
static void xen_9pfs_front_remove(struct xenbus_device *dev)
315313
{
316-
struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
314+
struct xen_9pfs_front_priv *priv;
317315

316+
write_lock(&xen_9pfs_lock);
317+
priv = dev_get_drvdata(&dev->dev);
318+
if (priv == NULL) {
319+
write_unlock(&xen_9pfs_lock);
320+
return;
321+
}
318322
dev_set_drvdata(&dev->dev, NULL);
323+
list_del(&priv->list);
324+
write_unlock(&xen_9pfs_lock);
325+
319326
xen_9pfs_front_free(priv);
320327
}
321328

@@ -382,7 +389,7 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
382389
{
383390
int ret, i;
384391
struct xenbus_transaction xbt;
385-
struct xen_9pfs_front_priv *priv = dev_get_drvdata(&dev->dev);
392+
struct xen_9pfs_front_priv *priv;
386393
char *versions, *v;
387394
unsigned int max_rings, max_ring_order, len = 0;
388395

@@ -410,6 +417,10 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
410417
if (p9_xen_trans.maxsize > XEN_FLEX_RING_SIZE(max_ring_order))
411418
p9_xen_trans.maxsize = XEN_FLEX_RING_SIZE(max_ring_order) / 2;
412419

420+
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
421+
if (!priv)
422+
return -ENOMEM;
423+
priv->dev = dev;
413424
priv->rings = kcalloc(XEN_9PFS_NUM_RINGS, sizeof(*priv->rings),
414425
GFP_KERNEL);
415426
if (!priv->rings) {
@@ -468,6 +479,11 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
468479
goto error;
469480
}
470481

482+
write_lock(&xen_9pfs_lock);
483+
dev_set_drvdata(&dev->dev, priv);
484+
list_add_tail(&priv->list, &xen_9pfs_devs);
485+
write_unlock(&xen_9pfs_lock);
486+
471487
xenbus_switch_state(dev, XenbusStateInitialised);
472488
return 0;
473489

@@ -482,19 +498,6 @@ static int xen_9pfs_front_init(struct xenbus_device *dev)
482498
static int xen_9pfs_front_probe(struct xenbus_device *dev,
483499
const struct xenbus_device_id *id)
484500
{
485-
struct xen_9pfs_front_priv *priv = NULL;
486-
487-
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
488-
if (!priv)
489-
return -ENOMEM;
490-
491-
priv->dev = dev;
492-
dev_set_drvdata(&dev->dev, priv);
493-
494-
write_lock(&xen_9pfs_lock);
495-
list_add_tail(&priv->list, &xen_9pfs_devs);
496-
write_unlock(&xen_9pfs_lock);
497-
498501
return 0;
499502
}
500503

0 commit comments

Comments
 (0)