Re: [RESEND] [PATCH] VFS: make file->f_pos access atomic on 32bit arch

[Linus Torvalds <torvalds@linux-foundation.org>]

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