Hello Dan Carpenter:
Sorry for the patch 41320b18a0e0 I submitted, I thought
put_dev(&tgt->dev) is not the same as kfree(tgt).
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:26, Dan Carpenter wrote:
Hello Zhu Wang,
The patch 41320b18a0e0: "scsi: snic: Fix possible memory leak if
device_add() fails" from Aug 1, 2023 (linux-next), leads to the
following Smatch static checker warning:
drivers/scsi/snic/snic_disc.c:311 snic_tgt_create()
warn: freeing device managed memory (UAF): 'tgt'
drivers/scsi/snic/snic_disc.c
234 static struct snic_tgt *
235 snic_tgt_create(struct snic *snic, struct snic_tgt_id *tgtid)
236 {
237 struct snic_tgt *tgt = NULL;
238 unsigned long flags;
239 int ret;
240
241 tgt = snic_tgt_lookup(snic, tgtid);
242 if (tgt) {
243 /* update the information if required */
244 return tgt;
245 }
246
247 tgt = kzalloc(sizeof(*tgt), GFP_KERNEL);
248 if (!tgt) {
249 SNIC_HOST_ERR(snic->shost, "Failure to allocate snic_tgt.\n");
250 ret = -ENOMEM;
251
252 return tgt;
253 }
254
255 INIT_LIST_HEAD(&tgt->list);
256 tgt->id = le32_to_cpu(tgtid->tgt_id);
257 tgt->channel = 0;
258
259 SNIC_BUG_ON(le16_to_cpu(tgtid->tgt_type) > SNIC_TGT_SAN);
260 tgt->tdata.typ = le16_to_cpu(tgtid->tgt_type);
261
262 /*
263 * Plugging into SML Device Tree
264 */
265 tgt->tdata.disc_id = 0;
266 tgt->state = SNIC_TGT_STAT_INIT;
267 device_initialize(&tgt->dev);
268 tgt->dev.parent = get_device(&snic->shost->shost_gendev);
269 tgt->dev.release = snic_tgt_dev_release;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This is a similar bug to the one I reported earlier.
270 INIT_WORK(&tgt->scan_work, snic_scsi_scan_tgt);
271 INIT_WORK(&tgt->del_work, snic_tgt_del);
272 switch (tgt->tdata.typ) {
273 case SNIC_TGT_DAS:
274 dev_set_name(&tgt->dev, "snic_das_tgt:%d:%d-%d",
275 snic->shost->host_no, tgt->channel, tgt->id);
276 break;
277
278 case SNIC_TGT_SAN:
279 dev_set_name(&tgt->dev, "snic_san_tgt:%d:%d-%d",
280 snic->shost->host_no, tgt->channel, tgt->id);
281 break;
282
283 default:
284 SNIC_HOST_INFO(snic->shost, "Target type Unknown Detected.\n");
285 dev_set_name(&tgt->dev, "snic_das_tgt:%d:%d-%d",
286 snic->shost->host_no, tgt->channel, tgt->id);
287 break;
288 }
289
290 spin_lock_irqsave(snic->shost->host_lock, flags);
291 list_add_tail(&tgt->list, &snic->disc.tgt_list);
292 tgt->scsi_tgt_id = snic->disc.nxt_tgt_id++;
293 tgt->state = SNIC_TGT_STAT_ONLINE;
294 spin_unlock_irqrestore(snic->shost->host_lock, flags);
295
296 SNIC_HOST_INFO(snic->shost,
297 "Tgt %d, type = %s detected. Adding..\n",
298 tgt->id, snic_tgt_type_to_str(tgt->tdata.typ));
299
300 ret = device_add(&tgt->dev);
301 if (ret) {
302 SNIC_HOST_ERR(snic->shost,
303 "Snic Tgt: device_add, with err = %d\n",
304 ret);
305
306 put_device(&tgt->dev);
^^^^^^^^^^^^^^^^^^^^
The put_device() will free tgt.
307 put_device(&snic->shost->shost_gendev);
308 spin_lock_irqsave(snic->shost->host_lock, flags);
309 list_del(&tgt->list);
Use after free
310 spin_unlock_irqrestore(snic->shost->host_lock, flags);
--> 311 kfree(tgt);
Double free.
312 tgt = NULL;
313
314 return tgt;
315 }
316
317 SNIC_HOST_INFO(snic->shost, "Scanning %s.\n", dev_name(&tgt->dev));
318
319 scsi_queue_work(snic->shost, &tgt->scan_work);
320
321 return tgt;
322 } /* end of snic_tgt_create */
regards,
dan carpenter
