Commit f03a6783 authored Aug 28, 2020 by Liam Mark

dma-buf: Cache the dma-buf inode number



Now that dma-buf has switched to handling its release from the
file_operations' release function to the dentry_operations' d_release
function there is a potential use-after-free when accessing the
buf_obj->file->f_inode as the path called by __fput eventually calls
__dentry_kill which frees the inode before calling the dentry_operations'
d_release function, so buf_obj->file->f_inode may no longer be valid.

It doesn't look like there is a clean way to check if the inode is valid.
For example using dentry->d_lock to look at the dentry inode would first
involve getting the dentry via a call to file_dentry which potentially
uses buf_obj->file->f_inode and it may no longer be valid.

It looks safer to simply cache the inode number since it is a useful value
to have as it identifies dma-buf objects.

KASAN log gave the following:
KASAN: use-after-free in dma_buf_debug_show+0x190/0x660
E : Read of size 8 at addr ffffff81051d7740 by task cat/19499
I Call trace:
I : dump_backtrace+0x0/0x2c8
I : show_stack+0x20/0x2c
I : dump_stack+0x154/0x1f4
I : print_address_description+0x84/0x580
I : __kasan_report+0x1b8/0x218
I : kasan_report+0x10/0x18
I : __asan_load8+0x90/0x98
I : dma_buf_debug_show+0x190/0x660
I : seq_read+0x2dc/0x830
I : full_proxy_read+0xbc/0x110
I : __vfs_read+0xd8/0x3d0
I : vfs_read+0x158/0x278
I : ksys_read+0x114/0x1e0
I : __arm64_sys_read+0x4c/0x5c
I : el0_svc_common+0x130/0x294
I : el0_svc_handler+0xdc/0xec
I : el0_svc+0x8/0xc
E : Allocated by task 912:
I : __kasan_kmalloc+0xfc/0x1b8
I : kasan_slab_alloc+0x14/0x1c
I : kmem_cache_alloc+0x320/0x3c4
I : new_inode_pseudo+0xbc/0x168
I : alloc_anon_inode+0x28/0x170
I : dma_buf_export+0x1e0/0x3ec
I : ion_dmabuf_alloc+0x168/0x1d4
I : ion_ioctl+0x24c/0x5c8
I : do_vfs_ioctl+0x55c/0xcb4
I : __arm64_sys_ioctl+0xa0/0xe4
I : el0_svc_common+0x130/0x294
I : el0_svc_handler+0xdc/0xec
I : el0_svc+0x8/0xc
E : Freed by task 40:
I : __kasan_slab_free+0x160/0x22c
I : kasan_slab_free+0x10/0x1c
I : slab_free_freelist_hook+0xdc/0x15c
I : kmem_cache_free+0xf8/0x350
I : i_callback+0x48/0x220
I : rcu_do_batch+0x2e8/0x594
I : nocb_cb_wait+0x158/0x930
I : rcu_nocb_cb_kthread+0x20/0x44
I : kthread+0x1ec/0x204

Change-Id: Ic3cfabf3524d2d34fe8b341e84f2ff259e4fad63
Signed-off-by: Liam Mark <lmark@codeaurora.org>

parent 2542e1ed

drivers/dma-buf/dma-buf.c

+3 −3

Original line number	Diff line number	Diff line
		@@ -63,8 +63,7 @@ static int dma_buf_invoke_dtor(struct dma_buf *dmabuf)
		ret = dmabuf->dtor(dmabuf, dmabuf->dtor_data);
		if (ret < 0)
		pr_warn_ratelimited("Leaking dmabuf ino: %ld because destructor failed error: %d\n",
		file_inode(dmabuf->file)->i_ino,
		ret);
		to_msm_dma_buf(dmabuf)->i_ino, ret);
		}

		return ret;
		@@ -587,6 +586,7 @@ struct dma_buf dma_buf_export(const struct dma_buf_export_info exp_info)
		ret = PTR_ERR(file);
		goto err_dmabuf;
		}
		msm_dma_buf->i_ino = file_inode(file)->i_ino;

		file->f_mode \|= FMODE_LSEEK;
		dmabuf->file = file;
		@@ -1288,7 +1288,7 @@ static int dma_buf_debug_show(struct seq_file s, void unused)
		buf_obj->file->f_flags, buf_obj->file->f_mode,
		file_count(buf_obj->file),
		buf_obj->exp_name,
		file_inode(buf_obj->file)->i_ino,
		to_msm_dma_buf(buf_obj)->i_ino,
		buf_obj->name ?: "");

		robj = buf_obj->resv;

include/linux/dma-buf.h

+2 −0

Original line number	Diff line number	Diff line
		@@ -427,10 +427,12 @@ struct dma_buf {
		* struct msm_dma_buf - Holds the meta data associated with a shared buffer
		* object, as well as the buffer object.
		* @refs: list entry for dma-buf reference tracking
		* @i_ino: inode number
		* @dma_buf: the shared buffer object
		*/
		struct msm_dma_buf {
		struct list_head refs;
		unsigned long i_ino;
		struct dma_buf dma_buf;
		};