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

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

On 2010-01-19 12:06, Jan Beulich wrote:
> >>> "Joe Jin" <joe.jin@xxxxxxxxxx> 19.01.10 12:32 >>>
> >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.
> 
> I think I understand what you're saying. But then there's only one
> solution - avoid freeing those two data structures from the .remove
> handler.

locked blk{back,tap}_remove and VBD_SHOW? I did not found any refcnt help 
for this. but even add lock for this, I think still need the checks.

> 
> >The checks may look like below?
> 
> Checking dev to be non-NULL is certainly pointless in any case.
> 
> But wait - how did you see the crash you're trying to fix occur in the
> first place? blkback_remove() calls xenvbd_sysfs_delif(), so the sysfs
> entries are gone by the time driver_data gets freed.

When tested continous reboot VM testcase kernel panic. then I write 
a program as blow:
while (1) {
        open("/sys/devices/xen-backend/vbd-x-xxxx/statistics/rd_sect");
        usleep(100);
        read(fd, buff, BUFF_LEN);
}

running it, then reboot the vm, will hit the panic.

As I have menthioned, when open the file, kernel have initialized 
file handler, when call file operation, call the function.
At here, even blkback_remove() have delete sysfs entry, but did not
released sysfs->fops, that caused the panic.


Thanks,
Joe

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