Re: [Xen-devel] [PATCH] Fix blkback/blktap sysfs read bug.

["Joe Jin" <joe.jin@xxxxxxxxxx>]

On 2010-01-19 10:25, Jan Beulich wrote:
> >>> "Joe Jin" <joe.jin@xxxxxxxxxx> 19.01.10 10:52 >>>
> >At backend driver blkback and blktap, when checking statistics information,
> >at the time vbd device remove, kernel will crash.
> >
> >Below patch will fix it, please review and apply.
> 
> This isn't a complete fix if I follow your analysis: There's still a race
> between blk{back,tap}_remove() freeing be->blkif/be and the sysfs
> code. dev->dev.driver_data (and possibly be->blkif) must be cleared
> before freeing it (them).
> 

Thanks for you point.
Anyway, I think need to add some check at VBD_SHOW even cleared 
dev->dev.driver_data before free it, sysfs->fops() have been 
initialized when call open(), and later when read the file, call
trace will fall VBD_SHOW defined function(s), so it should crashed.

The checks may look like below?


diff -r 6061d5615522 drivers/xen/blkback/xenbus.c
--- a/drivers/xen/blkback/xenbus.c      Fri Jan 08 13:07:17 2010 +0000
+++ b/drivers/xen/blkback/xenbus.c      Tue Jan 19 19:30:32 2010 +0800
@@ -104,10 +104,13 @@
                                   struct device_attribute *attr,       \
                                   char *buf)                           \
        {                                                               \
+               ssize_t ret = -ENODEV;                                  \
                struct xenbus_device *dev = to_xenbus_device(_dev);     \
-               struct backend_info *be = dev->dev.driver_data;         \
+               struct backend_info *be;                                \
                                                                        \
-               return sprintf(buf, format, ##args);                    \
+               if (dev && (be = dev->dev.driver_data) && be->blkif)    \
+                       ret = sprintf(buf, format, ##args);             \
+               return ret;                                             \
        }                                                               \
        static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
 
diff -r 6061d5615522 drivers/xen/blktap/xenbus.c
--- a/drivers/xen/blktap/xenbus.c       Fri Jan 08 13:07:17 2010 +0000
+++ b/drivers/xen/blktap/xenbus.c       Tue Jan 19 19:30:32 2010 +0800
@@ -122,10 +122,13 @@
                                   struct device_attribute *attr,       \
                                   char *buf)                           \
        {                                                               \
+               ssize_t ret = -ENODEV;                                  \
                struct xenbus_device *dev = to_xenbus_device(_dev);     \
-               struct backend_info *be = dev->dev.driver_data;         \
+               struct backend_info *be;                                \
                                                                        \
-               return sprintf(buf, format, ##args);                    \
+               if (dev && (be = dev->dev.driver_data) && be->blkif)    \
+                       ret = sprintf(buf, format, ##args);             \
+               return ret;                                             \
        }                                                               \
        static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel