linux-kernel - Re: Update of file offset on write() etc. is non-atomic with I/O

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]

Date:	Mon, 3 Mar 2014 22:09:05 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>,
	lkml <linux-kernel@...r.kernel.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	Theodore T'so <tytso@....edu>, Christoph Hellwig <hch@....de>,
	Chris Mason <clm@...com>, Dave Chinner <david@...morbit.com>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	"J. Bruce Fields" <bfields@...i.umich.edu>,
	Yongzhi Pan <panyongzhi@...il.com>
Subject: Re: Update of file offset on write() etc. is non-atomic with I/O

On Mon, Mar 03, 2014 at 01:56:31PM -0800, Linus Torvalds wrote:
> On Mon, Mar 3, 2014 at 1:45 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > Um...  That's odd - we *could* get there with f.need_put and
> > file_count(file) equal to 1, but why would we want to take
> > f_pos_lock in that case?
> 
> Because that means that the file table is shared among threads. So
> another thread can access the struct file pointer and do a concurrent
> read() or write() on it, and so we need to lock f_pos.

Yes, but in that case fdget() has grabbed a reference to that sucker,
so the only way to end with refcount 1 is to have the damn thing gone
from descriptor table in between.  And AFAICS in that case we are just
fine without f_pos_lock.

>  - concurrent access due to duplicated "struct file_table" pointers.
> This is the "need_put" test, since __fget_light() will have tested the
> proper files->count already.

Shared descriptor table means that we'd better have a reference grabbed
already...

I agree that file_count(file) > 1 for any reason requires locking it;
it's just that need_put means exactly that we have grabbed a reference
ourselves.  So having need_put && file_count <= 1 means that we have
grabbed it and then somebody dropped all other references.  Including
the ones in descriptor table(s), shared or not, etc.  In that case they'd
better not touch that struct file anymore - after all, once we are done
with whatever we are doing, we'll do fput() and it will be ripped from
under them with no warning.
--
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/