Skip to contentSkip to conversation

Talk_to_backend on xenbus_probe only if drv->probe was sucessful

PATCH Applied Murillo Bernardes 2 participants 4 messages Dec 12, 2005 – Dec 15, 2005

lore
Share & tools
lore

Murillo Bernardes

author
#0 lore raw

PATCH 1/1 Talk_to_backend on xenbus_probe only if drv->probe was sucessful

Hi,

This patch makes xenbus_probe only call talk_to_backend if drv->probe() was sucessful.

There is no sense in add a watch before drv->probe, because if it fails the otherend_changed callback will be called and receive a struct with invalid pointers and we get a segfault (ooops).

Signature · 2 lines
Murillo Fernandes Bernardes
IBM Linux Technology Center

# HG changeset patch
# User [email protected]
# Node ID 9ecd0d7fc6693e2aaceeb5e8c28af84109e9d3d8 # Parent bdcb115c667a12a5514517456639142c1273b0f1

Put watch on device node only if probe was sucessful.

This fix segfault on netfront when creating more than three network devices on domU.

Signed-off-by Murillo F. Bernardes <[email protected]>
linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c +15 -15
diff -r bdcb115c667a -r 9ecd0d7fc669 linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c
@@ -338,6 +338,21 @@
 
 	DPRINTK("");
 
+	if (!drv->probe) {
+		err = -ENODEV;
+		goto fail;
+	}
+
+	id = match_device(drv->ids, dev);
+	if (!id) {
+		err = -ENODEV;
+		goto fail;
+	}
+
+	err = drv->probe(dev, id);
+	if (err)
+		goto fail;
+
 	err = talk_to_otherend(dev);
 	if (err) {
 		printk(KERN_WARNING

@@ -345,21 +360,6 @@
 		       dev->nodename);
 		return err;
 	}
-
-	if (!drv->probe) {
-		err = -ENODEV;
-		goto fail;
-	}
-
-	id = match_device(drv->ids, dev);
-	if (!id) {
-		err = -ENODEV;
-		goto fail;
-	}
-
-	err = drv->probe(dev, id);
-	if (err)
-		goto fail;
 
 	return 0;
 fail:

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Applied
#1 lore raw
Quoted 8 lines · Murillo Bernardes

Hi,

This patch makes xenbus_probe only call talk_to_backend if drv->probe() was sucessful.

There is no sense in add a watch before drv->probe, because if it fails the otherend_changed callback will be called and receive a struct with invalid pointers and we get a segfault (ooops).

Applied, thanks Murillo.

Ewan.

author
#2 lore raw
Quoted 6 lines · Ewan Mellor
Quoted 5 lines · Murillo Bernardes

Hi,

This patch makes xenbus_probe only call talk_to_backend if drv->probe() was sucessful.

Quoted 7 lines
Quoted 3 lines

There is no sense in add a watch before drv->probe, because if it fails the otherend_changed callback will be called and receive a struct with invalid pointers and we get a segfault (ooops).

Applied, thanks Murillo.

Ewan.

I just found a problem on that. It breaks block devices! I don't know why yet.

Do you know why block devices rely on that watch before probe?

Thanks

Signature · 2 lines
Murillo Fernandes Bernardes
IBM Linux Technology Center
#3 lore raw
Quoted 11 lines · Murillo Bernardes
Quoted 10 lines · Ewan Mellor
Quoted 8 lines · Murillo Bernardes

Hi,

This patch makes xenbus_probe only call talk_to_backend if drv->probe() was sucessful.

There is no sense in add a watch before drv->probe, because if it fails the otherend_changed callback will be called and receive a struct with invalid pointers and we get a segfault (ooops).

Quoted 6 lines
Quoted 3 lines

Applied, thanks Murillo.

Ewan.

I just found a problem on that. It breaks block devices! I don't know why yet.

Do you know why block devices rely on that watch before probe?

Yes, I found that one too! blkback_probe calls alloc_blkif(dev->otherend_id), so it doesn't need the watch, but it does need the otherend details to have been read. I'm applying another patch right now.

Ewan.