Am 16.05.22 um 09:13 schrieb Charan Teja Kalla:
++ Adding Christian On 5/16/2022 12:19 PM, Dan Carpenter wrote:Hello Charan Teja Reddy, The patch ef3a6b70507a: "dma-buf: call dma_buf_stats_setup after dmabuf is in valid list" from May 10, 2022, leads to the following Smatch static checker warning: drivers/dma-buf/dma-buf.c:569 dma_buf_export() warn: '&dmabuf->list_node' not removed from list drivers/dma-buf/dma-buf.c 538 file = dma_buf_getfile(dmabuf, exp_info->flags); 539 if (IS_ERR(file)) { 540 ret = PTR_ERR(file); 541 goto err_dmabuf; 542 } 543 544 file->f_mode |= FMODE_LSEEK; 545 dmabuf->file = file; 546 547 mutex_init(&dmabuf->lock); 548 INIT_LIST_HEAD(&dmabuf->attachments); 549 550 mutex_lock(&db_list.lock); 551 list_add(&dmabuf->list_node, &db_list.head); Added to the list 552 mutex_unlock(&db_list.lock); 553 554 ret = dma_buf_stats_setup(dmabuf); 555 if (ret) 556 goto err_sysfs; Goto 557 558 return dmabuf; 559 560 err_sysfs: 561 /* 562 * Set file->f_path.dentry->d_fsdata to NULL so that when 563 * dma_buf_release() gets invoked by dentry_ops, it exits 564 * early before calling the release() dma_buf op. 565 */ 566 file->f_path.dentry->d_fsdata = NULL; 567 fput(file); 568 err_dmabuf: 569 kfree(dmabuf); dmabuf is freed, but it's still on the list so it leads to a use after free.This seems to be a false positive. On closing the file @line no:567, it ends up calling dma_buf_file_release() which does remove dmabuf from its list.
Yeah, correct as far as I can see. The checker just can't see that the fput will cleanup the list.
Regards, Christian.
static int dma_buf_file_release(struct inode *inode, struct file *file) { ...... mutex_lock(&db_list.lock); list_del(&dmabuf->list_node); mutex_unlock(&db_list.lock); ...... }570 err_module: 571 module_put(exp_info->owner); 572 return ERR_PTR(ret); 573 } regards, dan carpenter
