| Message ID | 1469100724-11039-1-git-send-email-asavkov@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21 Jul 2016, at 7:32, Artem Savkov wrote: > When bl_parse_deviceid() fails in bl_alloc_deviceid_node() on > blkdev_get_by_*() step we get an pnfs_block_dev struct that is > uninitialized except for bdev field which is set to whatever error > blkdev_get_by_*() returns. bl_free_device() then tries to call > blkdev_put() if bdev is not 0 resulting in a wrong pointer > dereference. > > Fixing this by setting bdev in struct pnfs_block_dev only if we didn't > get an error from blkdev_get_by_*(). > > Signed-off-by: Artem Savkov <asavkov@redhat.com> Looks good to me. Reviewed-by: Benjamin Coddington <bcodding@redhat.com> > --- > fs/nfs/blocklayout/dev.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c > index 118252f..a69ef4e 100644 > --- a/fs/nfs/blocklayout/dev.c > +++ b/fs/nfs/blocklayout/dev.c > @@ -235,18 +235,20 @@ bl_parse_simple(struct nfs_server *server, > struct pnfs_block_dev *d, > struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask) > { > struct pnfs_block_volume *v = &volumes[idx]; > + struct block_device *bdev; > dev_t dev; > > dev = bl_resolve_deviceid(server, v, gfp_mask); > if (!dev) > return -EIO; > > - d->bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_WRITE, NULL); > - if (IS_ERR(d->bdev)) { > + bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_WRITE, NULL); > + if (IS_ERR(bdev)) { > printk(KERN_WARNING "pNFS: failed to open device %d:%d (%ld)\n", > - MAJOR(dev), MINOR(dev), PTR_ERR(d->bdev)); > - return PTR_ERR(d->bdev); > + MAJOR(dev), MINOR(dev), PTR_ERR(bdev)); > + return PTR_ERR(bdev); > } > + d->bdev = bdev; > > > d->len = i_size_read(d->bdev->bd_inode); > @@ -350,17 +352,19 @@ bl_parse_scsi(struct nfs_server *server, struct > pnfs_block_dev *d, > struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask) > { > struct pnfs_block_volume *v = &volumes[idx]; > + struct block_device *bdev; > const struct pr_ops *ops; > int error; > > if (!bl_validate_designator(v)) > return -EINVAL; > > - d->bdev = bl_open_dm_mpath_udev_path(v); > - if (IS_ERR(d->bdev)) > - d->bdev = bl_open_udev_path(v); > - if (IS_ERR(d->bdev)) > - return PTR_ERR(d->bdev); > + bdev = bl_open_dm_mpath_udev_path(v); > + if (IS_ERR(bdev)) > + bdev = bl_open_udev_path(v); > + if (IS_ERR(bdev)) > + return PTR_ERR(bdev); > + d->bdev = bdev; > > d->len = i_size_read(d->bdev->bd_inode); > d->map = bl_map_simple; > -- > 2.5.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/blocklayout/dev.c b/fs/nfs/blocklayout/dev.c index 118252f..a69ef4e 100644 --- a/fs/nfs/blocklayout/dev.c +++ b/fs/nfs/blocklayout/dev.c @@ -235,18 +235,20 @@ bl_parse_simple(struct nfs_server *server, struct pnfs_block_dev *d, struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask) { struct pnfs_block_volume *v = &volumes[idx]; + struct block_device *bdev; dev_t dev; dev = bl_resolve_deviceid(server, v, gfp_mask); if (!dev) return -EIO; - d->bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_WRITE, NULL); - if (IS_ERR(d->bdev)) { + bdev = blkdev_get_by_dev(dev, FMODE_READ | FMODE_WRITE, NULL); + if (IS_ERR(bdev)) { printk(KERN_WARNING "pNFS: failed to open device %d:%d (%ld)\n", - MAJOR(dev), MINOR(dev), PTR_ERR(d->bdev)); - return PTR_ERR(d->bdev); + MAJOR(dev), MINOR(dev), PTR_ERR(bdev)); + return PTR_ERR(bdev); } + d->bdev = bdev; d->len = i_size_read(d->bdev->bd_inode); @@ -350,17 +352,19 @@ bl_parse_scsi(struct nfs_server *server, struct pnfs_block_dev *d, struct pnfs_block_volume *volumes, int idx, gfp_t gfp_mask) { struct pnfs_block_volume *v = &volumes[idx]; + struct block_device *bdev; const struct pr_ops *ops; int error; if (!bl_validate_designator(v)) return -EINVAL; - d->bdev = bl_open_dm_mpath_udev_path(v); - if (IS_ERR(d->bdev)) - d->bdev = bl_open_udev_path(v); - if (IS_ERR(d->bdev)) - return PTR_ERR(d->bdev); + bdev = bl_open_dm_mpath_udev_path(v); + if (IS_ERR(bdev)) + bdev = bl_open_udev_path(v); + if (IS_ERR(bdev)) + return PTR_ERR(bdev); + d->bdev = bdev; d->len = i_size_read(d->bdev->bd_inode); d->map = bl_map_simple;
When bl_parse_deviceid() fails in bl_alloc_deviceid_node() on blkdev_get_by_*() step we get an pnfs_block_dev struct that is uninitialized except for bdev field which is set to whatever error blkdev_get_by_*() returns. bl_free_device() then tries to call blkdev_put() if bdev is not 0 resulting in a wrong pointer dereference. Fixing this by setting bdev in struct pnfs_block_dev only if we didn't get an error from blkdev_get_by_*(). Signed-off-by: Artem Savkov <asavkov@redhat.com> --- fs/nfs/blocklayout/dev.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)