On Tue, Jun 11, 2013 at 12:52:36AM +0100, Filipe David Borba Manana wrote: > If a device could not be opened in volumes.c:read_one_dev(), a > btrfs_device instance was allocated and added to the list of > devices of the fs - however this device instance had its fd, > name and label fields not initialized. This is problematic in > disk-io.c:close_all_devices() as it tried to sync, fadvise and > close the (invalid) fd of the device, and kfree() its name and > label, which pointed to random memory locations. > > Thread 1 (Thread 0x7f0a3d2d1740 (LWP 23585)): > #0 __GI___libc_free (mem=0xa5a5a5a5a5a5a5a5) at malloc.c:2970 > #1 0x000000000042054b in close_all_devices (fs_info=0x1e92bf0) at > disk-io.c:1276 > #2 0x0000000000421dcd in close_ctree (root=<optimized out>) at > disk-io.c:1336 > #3 0x0000000000418cfa in cmd_check (argc=<optimized out>, argv=<optimized > out>) at cmds-check.c:4171 > #4 0x0000000000403ed4 in main (argc=2, argv=0x7fff9a583d28) at btrfs.c:295
Good catch! This addresses one of my problems. Reviewed-by: Liu Bo <bo.li....@oracle.com> thanks, liubo > > Signed-off-by: Filipe David Borba Manana <fdman...@gmail.com> > --- > disk-io.c | 4 ++-- > volumes.c | 5 +++-- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/disk-io.c b/disk-io.c > index 21b410d..bd9cf4e 100644 > --- a/disk-io.c > +++ b/disk-io.c > @@ -1267,12 +1267,12 @@ static int close_all_devices(struct btrfs_fs_info > *fs_info) > while (!list_empty(list)) { > device = list_entry(list->next, struct btrfs_device, dev_list); > list_del_init(&device->dev_list); > - if (device->fd) { > + if (device->fd >= 0) { > fsync(device->fd); > if (posix_fadvise(device->fd, 0, 0, > POSIX_FADV_DONTNEED)) > fprintf(stderr, "Warning, could not drop > caches\n"); > + close(device->fd); > } > - close(device->fd); > kfree(device->name); > kfree(device->label); > kfree(device); > diff --git a/volumes.c b/volumes.c > index d6f81f8..061f094 100644 > --- a/volumes.c > +++ b/volumes.c > @@ -116,6 +116,7 @@ static int device_list_add(const char *path, > /* we can safely leave the fs_devices entry around */ > return -ENOMEM; > } > + device->fd = -1; > device->devid = devid; > memcpy(device->uuid, disk_super->dev_item.uuid, > BTRFS_UUID_SIZE); > @@ -1628,10 +1629,10 @@ static int read_one_dev(struct btrfs_root *root, > if (!device) { > printk("warning devid %llu not found already\n", > (unsigned long long)devid); > - device = kmalloc(sizeof(*device), GFP_NOFS); > + device = kzalloc(sizeof(*device), GFP_NOFS); > if (!device) > return -ENOMEM; > - device->total_ios = 0; > + device->fd = -1; > list_add(&device->dev_list, > &root->fs_info->fs_devices->devices); > } > -- > 1.7.9.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html