[RFC] deadlock in "drm/exynos: fix wrong pointer access at vm close"

[Inki Dae]

Hi,

> -----Original Message-----
> From: Al Viro [mailto:viro at ftp.linux.org.uk] On Behalf Of Al Viro
> Sent: Monday, September 23, 2013 6:29 AM
> To: YoungJun Cho
> Cc: dri-devel at lists.freedesktop.org; Inki Dae
> Subject: [RFC] deadlock in "drm/exynos: fix wrong pointer access at vm
> close"
> 
> 	You have drm_dev->struct_mutex grabbed before ->mmap_sem in
> exynos_drm_gem_mmap_ioctl() and after - in exynos_drm_gem_fault()
> (since ->fault() is always called with ->mmap_sem held).  Looks like
> a garden-variety AB-BA deadlock...
> 

Right, if mmap system call is requested by another process between ->f_op
pointer changing and restoring, the deadlock can be incurred.

For this, I think we can resolve this issue like below,

At exynos_drm_gem_mmap_ioctl()
	down_write(&mm->mmap_sem);
	mutex_lock(&dev->struct_mutex);
	...
	addr = security_mmap_file(...);
	if (!addr) {
		unsigned long populate;



		addr = do_mmap_pgoff(...);
		if (populate)
			mm_populater(...);
	}
	...
	mutex_unlock(&dev->struct_mutex);
	up_write(&mm->mmap_sem);


The point above is to call down_write(&mm->mmap_sem) before do_mmap_pgoff,
and then to call up_write(...) after do_mmap_pg_off so that when another
process called mmap system call, the process can wait for up_write(...)
until the ->f_op pointer is restored.

I will post this patch soon. other opinions?

Thanks,
Inki Dae

> 	Incidentally, what should happen if another process shares the
> same opened file (e.g. inherited over fork()) and does mmap() just
> as we have ->f_op switched?