linux-kernel - Re: [RESEND] [PATCH] VFS: make file->f

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: <alpine.LFD.2.00.0810090744110.3210@nehalem.linux-foundation.org>
Date:	Thu, 9 Oct 2008 07:58:53 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Matthew Wilcox <matthew@....cx>
cc:	Pavel Machek <pavel@...e.cz>,
	Nick Piggin <nickpiggin@...oo.com.au>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <andi@...stfloor.org>,
	Hisashi Hifumi <hifumi.hisashi@....ntt.co.jp>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	"Theodore Ts'o" <tytso@....edu>
Subject: Re: [RESEND] [PATCH] VFS: make file->f_pos access atomic on 32bit
 arch

On Thu, 9 Oct 2008, Matthew Wilcox wrote:
> On Thu, Oct 09, 2008 at 02:23:19PM +0200, Pavel Machek wrote:
> >
> > We have append-only files, and normal users should not be able to work
> > around that restriction.
> 
> Is it possible to work around this restriction by exploiting this?

No, I don't think it is.

Because we had various nasty races, and various broken filesystems using 
"f->f_pos" directly (and then pread/pwrite not working), we fixed things 
many years ago, and nobody should use "f_pos" directly any more for any 
regular file access.

Oh, you'll see a _lot_ of f_pos accesses if you grep for them in low-level 
filesystems, but they should be for directory accesses, that are all under 
i_mutex. And O_APPEND obviously doesn't enter into it anyway.

So for regular IO, all the filesystems should never touch f_pos directly 
at all, they only ever touch a local "pos" that gets cached, and then at 
the end of the write sys_write() will write it back with file_pos_write(). 
That function was done exactly so that we _could_ do locking if we cared. 
Nobody ever did.

So even though filesystems get passed a _pointer_ to the position, it's 
all actually a pointer to just a private per-thread, on-stack entry.

The reason for that is that we really used to have bugs where the 
low-level filesystem assumed that "*pos" didn't change from under it while 
the access was going on (reading it multiple times and comparing against 
i_size etc), and exactly due to things like O_APPEND races against lseek.

So I think f_pos is fine. Yes, yes, if two threads or processes access the 
same file pointer concurrently, that means that f_pos at the end may be 
crazy, but that really is true regardless of whether you are able to hit 
the *very* small race of updating the 32-bit lower/upper fields in some 
mixed manner. No sane user program can possibly really care, since it 
would already be getting random offsets.

(Yeah, yeah, I could see some really crazy code that can do retries with 
optimistic locking in user space and could possibly see this as a bug, but 
that really is totally insane code, and I doubt you could write such a 
crazy thing to actually work).

			Linus
--
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/