BACKPORT: dma-buf: Move dma_buf_release() from fops to dentry_ops
Charan Teja reported a 'use-after-free' in dmabuffs_dname [1], which happens if the dma_buf_release() is called while the userspace is accessing the dma_buf pseudo fs's dmabuffs_dname() in another process, and dma_buf_release() releases the dmabuf object when the last reference to the struct file goes away. I discussed with Arnd Bergmann, and he suggested that rather than tying the dma_buf_release() to the file_operations' release(), we can tie it to the dentry_operations' d_release(), which will be called when the last ref to the dentry is removed. The path exercised by __fput() calls f_op->release() first, and then calls dput, which eventually calls d_op->d_release(). In the 'normal' case, when no userspace access is happening via dma_buf pseudo fs, there should be exactly one fd, file, dentry and inode, so closing the fd will kill of everything right away. In the presented case, the dentry's d_release() will be called only when the dentry's last ref is released. Therefore, lets move dma_buf_release() from fops->release() to d_ops->d_release() Many thanks to Arnd for his FS insights :) [1]: https://lore.kernel.org/patchwork/patch/1238278/ Fixes: bb2bb90 ("dma-buf: add DMA_BUF_SET_NAME ioctls") Reported-by: syzbot+3643a18836bce555bff6@syzkaller.appspotmail.com Cc: <stable@vger.kernel.org> [5.3+] Cc: Arnd Bergmann <arnd@arndb.de> Reported-by: Charan Teja Reddy <charante@codeaurora.org> Reviewed-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org> Tested-by: Charan Teja Reddy <charante@codeaurora.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200611114418.19852-1-sumit.semwal@linaro.org Bug: 162699017 Signed-off-by: Allen Chiu <allen.chiu@mediatek.com> Change-Id: Ief19296f201132c3e32b11958a857798c34f81fb Git-commit: 2581e5be6555232e784487fc00961ce83c5236a1 Git-repo: https://android.googlesource.com/kernel/msm Signed-off-by: PavanKumar S.R. <pavasr@codeaurora.org>
This commit is contained in:
parent
46f5f96602
commit
7681a073f9
1 changed files with 22 additions and 27 deletions
|
@ -102,35 +102,12 @@ out:
|
|||
dentry->d_name.name, ret > 0 ? name : "");
|
||||
}
|
||||
|
||||
static const struct dentry_operations dma_buf_dentry_ops = {
|
||||
.d_dname = dmabuffs_dname,
|
||||
};
|
||||
|
||||
static struct vfsmount *dma_buf_mnt;
|
||||
|
||||
static struct dentry *dma_buf_fs_mount(struct file_system_type *fs_type,
|
||||
int flags, const char *name, void *data)
|
||||
{
|
||||
return mount_pseudo(fs_type, "dmabuf:", NULL, &dma_buf_dentry_ops,
|
||||
DMA_BUF_MAGIC);
|
||||
}
|
||||
|
||||
static struct file_system_type dma_buf_fs_type = {
|
||||
.name = "dmabuf",
|
||||
.mount = dma_buf_fs_mount,
|
||||
.kill_sb = kill_anon_super,
|
||||
};
|
||||
|
||||
static int dma_buf_release(struct inode *inode, struct file *file)
|
||||
static void dma_buf_release(struct dentry *dentry)
|
||||
{
|
||||
struct dma_buf *dmabuf;
|
||||
struct dentry *dentry = file->f_path.dentry;
|
||||
int dtor_ret = 0;
|
||||
|
||||
if (!is_dma_buf_file(file))
|
||||
return -EINVAL;
|
||||
|
||||
dmabuf = file->private_data;
|
||||
dmabuf = dentry->d_fsdata;
|
||||
|
||||
spin_lock(&dentry->d_lock);
|
||||
dentry->d_fsdata = NULL;
|
||||
|
@ -167,9 +144,28 @@ static int dma_buf_release(struct inode *inode, struct file *file)
|
|||
|
||||
module_put(dmabuf->owner);
|
||||
dmabuf_dent_put(dmabuf);
|
||||
return 0;
|
||||
}
|
||||
|
||||
static const struct dentry_operations dma_buf_dentry_ops = {
|
||||
.d_dname = dmabuffs_dname,
|
||||
.d_release = dma_buf_release,
|
||||
};
|
||||
|
||||
static struct vfsmount *dma_buf_mnt;
|
||||
|
||||
static struct dentry *dma_buf_fs_mount(struct file_system_type *fs_type,
|
||||
int flags, const char *name, void *data)
|
||||
{
|
||||
return mount_pseudo(fs_type, "dmabuf:", NULL, &dma_buf_dentry_ops,
|
||||
DMA_BUF_MAGIC);
|
||||
}
|
||||
|
||||
static struct file_system_type dma_buf_fs_type = {
|
||||
.name = "dmabuf",
|
||||
.mount = dma_buf_fs_mount,
|
||||
.kill_sb = kill_anon_super,
|
||||
};
|
||||
|
||||
static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
|
||||
{
|
||||
struct dma_buf *dmabuf;
|
||||
|
@ -488,7 +484,6 @@ static void dma_buf_show_fdinfo(struct seq_file *m, struct file *file)
|
|||
}
|
||||
|
||||
static const struct file_operations dma_buf_fops = {
|
||||
.release = dma_buf_release,
|
||||
.mmap = dma_buf_mmap_internal,
|
||||
.llseek = dma_buf_llseek,
|
||||
.poll = dma_buf_poll,
|
||||
|
|
Loading…
Reference in a new issue