[bug report] scsi: core: Fix possible memory leak if device_add() fails

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello Zhu Wang,

The patch 04b5b5cb0136: "scsi: core: Fix possible memory leak if
device_add() fails" from Aug 3, 2023 (linux-next), leads to the
following Smatch static checker warning:

	drivers/scsi/raid_class.c:255 raid_component_add()
	warn: freeing device managed memory (UAF): 'rc'

drivers/scsi/raid_class.c
   212  static void raid_component_release(struct device *dev)
   213  {
   214          struct raid_component *rc =
   215                  container_of(dev, struct raid_component, dev);
   216          dev_printk(KERN_ERR, rc->dev.parent, "COMPONENT RELEASE\n");
   217          put_device(rc->dev.parent);
   218          kfree(rc);
   219  }
   220  
   221  int raid_component_add(struct raid_template *r,struct device *raid_dev,
   222                         struct device *component_dev)
   223  {
   224          struct device *cdev =
   225                  attribute_container_find_class_device(&r->raid_attrs.ac,
   226                                                        raid_dev);
   227          struct raid_component *rc;
   228          struct raid_data *rd = dev_get_drvdata(cdev);
   229          int err;
   230  
   231          rc = kzalloc(sizeof(*rc), GFP_KERNEL);
   232          if (!rc)
   233                  return -ENOMEM;
   234  
   235          INIT_LIST_HEAD(&rc->node);
   236          device_initialize(&rc->dev);

The comments for device_initialize() say we cannot call kfree(rc) after
this point.

   237          rc->dev.release = raid_component_release;
                                  ^^^^^^^^^^^^^^^^^^^^^^^
>From this point on calling put_device(&rc->dev) is the same as calling
raid_component_release(&rc->dev);

   238          rc->dev.parent = get_device(component_dev);
   239          rc->num = rd->component_count++;
   240  
   241          dev_set_name(&rc->dev, "component-%d", rc->num);
   242          list_add_tail(&rc->node, &rd->component_list);
   243          rc->dev.class = &raid_class.class;
   244          err = device_add(&rc->dev);
   245          if (err)
   246                  goto err_out;
   247  
   248          return 0;
   249  
   250  err_out:
   251          put_device(&rc->dev);
   252          list_del(&rc->node);
   253          rd->component_count--;
   254          put_device(component_dev);
   255          kfree(rc);

So this code is clearly a double free.  However, fixing it is not
obvious because why does raid_component_release() not call?

	list_del(&rc->node);
	rd->component_count--;

   256          return err;
   257  }

regards,
dan carpenter



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux