| Message ID | 20220805053319.3865-1-liubo03@inspur.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | dax: Check dev_set_name() return value | expand |
On Fri, Aug 05, 2022 at 01:33:19AM -0400, Bo Liu wrote: > It's possible that dev_set_name() returns -ENOMEM, catch and handle this. Did this cause a bug or some other problem when the name was not set? I think it is an ok change but without digging into the code I'm not clear why you did this. Ira > > Signed-off-by: Bo Liu <liubo03@inspur.com> > --- > drivers/dax/bus.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index 1dad813ee4a6..36cf245ee467 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c > @@ -765,7 +765,12 @@ static int devm_register_dax_mapping(struct dev_dax *dev_dax, int range_id) > device_initialize(dev); > dev->parent = &dev_dax->dev; > dev->type = &dax_mapping_type; > - dev_set_name(dev, "mapping%d", mapping->id); > + rc = dev_set_name(dev, "mapping%d", mapping->id); > + if (rc) { > + kfree(mapping); > + return rc; > + } > + > rc = device_add(dev); > if (rc) { > put_device(dev); > @@ -1334,7 +1339,9 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data) > dev_dax->region = dax_region; > dev = &dev_dax->dev; > device_initialize(dev); > - dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id); > + rc = dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id); > + if (rc) > + goto err_range; > > rc = alloc_dev_dax_range(dev_dax, dax_region->res.start, data->size); > if (rc) > -- > 2.27.0 > >
Bo Liu wrote: > It's possible that dev_set_name() returns -ENOMEM, catch and handle this. > > Signed-off-by: Bo Liu <liubo03@inspur.com> > --- > drivers/dax/bus.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c > index 1dad813ee4a6..36cf245ee467 100644 > --- a/drivers/dax/bus.c > +++ b/drivers/dax/bus.c > @@ -765,7 +765,12 @@ static int devm_register_dax_mapping(struct dev_dax *dev_dax, int range_id) > device_initialize(dev); > dev->parent = &dev_dax->dev; > dev->type = &dax_mapping_type; > - dev_set_name(dev, "mapping%d", mapping->id); > + rc = dev_set_name(dev, "mapping%d", mapping->id); > + if (rc) { > + kfree(mapping); No, this needs to trigger put_device() otherwise it leaks mapping->id.
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c index 1dad813ee4a6..36cf245ee467 100644 --- a/drivers/dax/bus.c +++ b/drivers/dax/bus.c @@ -765,7 +765,12 @@ static int devm_register_dax_mapping(struct dev_dax *dev_dax, int range_id) device_initialize(dev); dev->parent = &dev_dax->dev; dev->type = &dax_mapping_type; - dev_set_name(dev, "mapping%d", mapping->id); + rc = dev_set_name(dev, "mapping%d", mapping->id); + if (rc) { + kfree(mapping); + return rc; + } + rc = device_add(dev); if (rc) { put_device(dev); @@ -1334,7 +1339,9 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data) dev_dax->region = dax_region; dev = &dev_dax->dev; device_initialize(dev); - dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id); + rc = dev_set_name(dev, "dax%d.%d", dax_region->id, dev_dax->id); + if (rc) + goto err_range; rc = alloc_dev_dax_range(dev_dax, dax_region->res.start, data->size); if (rc)
It's possible that dev_set_name() returns -ENOMEM, catch and handle this. Signed-off-by: Bo Liu <liubo03@inspur.com> --- drivers/dax/bus.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)