Re: [bug report] dma-buf: call dma_buf_stats_setup after dmabuf is in valid list

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

 



++ 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.

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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux