Messages in this thread | | | Date | Mon, 11 May 2020 16:08:59 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v7 09/18] pstore/blk: Introduce backend for block devices |
| |
On Mon, May 11, 2020 at 04:36:51PM +0800, WeiXiong Liao wrote: > On 2020/5/11 AM 4:24, Kees Cook wrote: > > [...] > > +static struct block_device *psblk_get_bdev(void *holder, > > + struct bdev_info *info) > > Well. That's pretty a good idea to get information about block device > after registering. And after your codes, the global variable g_bdev_info is > useless. It's time to drop it.
Ah yes! I meant to clean that up and forgot. Fixed now.
> > [...] > > + bdev = blkdev_get_by_path(blkdev, mode, holder); > > + if (IS_ERR(bdev)) { > > + dev_t devt; > > + > > + devt = name_to_dev_t(blkdev); > > + if (devt == 0) > > + return ERR_PTR(-ENODEV); > > + bdev = blkdev_get_by_dev(devt, mode, holder); > > + } > > We should check bdev here. Otherwise, part_nr_sects_read() > may catch segment error. > > if (IS_ERR(bdev)) > return bdev;
Whoops, yes. Fixed.
> > + bdev = psblk_get_bdev(holder, &binfo); > > + if (IS_ERR(bdev)) { > > + pr_err("failed to open '%s'!\n", blkdev); > > + ret = PTR_ERR(bdev); > > + goto err_put_bdev; > > It should not goto err_put_bdev since bdev already be put if get_bdev() > fail.
Ah yes, good point. Fixed.
-- Kees Cook
| |