Messages in this thread | | | Subject | Re: [PATCH] nvdimm/pmem: Fix an error handling path in 'pmem_attach_disk()' | From | Marion & Christophe JAILLET <> | Date | Sun, 7 Nov 2021 18:25:15 +0100 |
| |
Le 07/11/2021 à 18:20, Christophe JAILLET a écrit : > Le 07/11/2021 à 18:11, Ira Weiny a écrit : >> On Sat, Nov 06, 2021 at 06:27:11PM +0100, Christophe JAILLET wrote: >>> If 'devm_init_badblocks()' fails, a previous 'blk_alloc_disk()' call >>> must >>> be undone. >> >> I think this is a problem... >> >>> >>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >>> --- >>> This patch is speculative. Several fixes on error handling paths have >>> been >>> done recently, but this one has been left as-is. There was maybe a good >>> reason that I have missed for that. So review with care! >>> >>> I've not been able to identify a Fixes tag that please me :( >>> --- >>> drivers/nvdimm/pmem.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c >>> index fe7ece1534e1..c37a1e6750b3 100644 >>> --- a/drivers/nvdimm/pmem.c >>> +++ b/drivers/nvdimm/pmem.c >>> @@ -490,8 +490,9 @@ static int pmem_attach_disk(struct device *dev, >>> nvdimm_namespace_disk_name(ndns, disk->disk_name); >>> set_capacity(disk, (pmem->size - pmem->pfn_pad - >>> pmem->data_offset) >>> / 512); >>> - if (devm_init_badblocks(dev, &pmem->bb)) >>> - return -ENOMEM; >>> + rc = devm_init_badblocks(dev, &pmem->bb); >>> + if (rc) >>> + goto out; >> >> But I don't see this 'out' label in the function currently? Was that >> part of >> your patch missing? > > Hi, > the patch is based on the latest linux-next. > See [1]. The 'out' label exists there and is already used. > > In fact, I run an own-made coccinelle script which tries to spot mix-up > between return and goto. > In this case, we have a 'return -ENOMEM' after a 'goto out' which looks > spurious. Hence, my patch. > > [1]:https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/nvdimm/pmem.c#n512 >
Lol, the #n512 above is in fact another place that should be updated as well. I missed it and only fixed #n494!
CJ
> > CJ > >> >> Ira >> >>> nvdimm_badblocks_populate(nd_region, &pmem->bb, &bb_range); >>> disk->bb = &pmem->bb; >>> -- >>> 2.30.2 >>> >> > >
| |