Hello Dan Carpenter:
Sorry for the patch 04b5b5cb0136 I submitted, I thought
put_dev(&rc->dev) is not the same as kfree(rc).
Then should I submit a revert patch again, or you can fix it yourself?
please let me know what I can do.
Sorry for the inconvenience again.
Best regards,
Zhu Wang.
On 2023/8/10 18:19, Dan Carpenter wrote:
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
