linux-kernel - Re: [PATCH 1/5] mmap: don't assume f_op->mmap() doesn't change vma->vm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49E544DF.2060109@kernel.org>
Date:	Wed, 15 Apr 2009 11:22:23 +0900
From:	Tejun Heo <tj@...nel.org>
To:	Hugh Dickins <hugh@...itas.com>
CC:	linux-kernel@...r.kernel.org, fuse-devel@...ts.sourceforge.net,
	miklos@...redi.hu, akpm@...ux-foundation.org, npiggin@...e.de
Subject: Re: [PATCH 1/5] mmap: don't assume f_op->mmap() doesn't change vma->vm_file

Hello,

Hugh Dickins wrote:
> On Tue, 14 Apr 2009, Tejun Heo wrote:
> 
>> mmap_region() assumes that vma->vm_file isn't changed by f_op->mmap()
>> and continues to use cache file after f_op->mmap() returns.
> 
> It does use "file" again in the unmap_and_free_vma error path
> (isn't that reasonable? if the ->mmap failed, it shouldn't have
> mucked with vma; and even if it has, then we'd better not change
> the current behaviour of which to fput), but I don't see where else.
> 
> Further down, covering both vma->vm_file previously set and previously
> unset cases, there is a "file = vma->vm_file;" before file is used.
> So I think this patch is not necessary - if it is necessary, it's
> already a bug, because already we switch from /dev/zero to a
> shmem file there.

Right, ->mmap() shouldn't modify @vma if it has failed.  I was trying
to clarify that @vma->file may change as the code was a bit confusing
regarding whether @vma->vm_file is allowed to be substituted or not.
Hmmmm... how about adding a BUG_ON() or WARN_ON() in the failure path
to make sure @vma->vm_file hasn't changed?

Anyways, I'm dropping this patch and updating FUSE mmap implementation
accordingly.  Thanks for the comment.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/