linux-ext4 - Re: [RESEND] [PATCH] lseek: change i

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 15 Jan 2009 09:21:13 -0500
From:	Theodore Tso <tytso@....edu>
To:	Matthew Wilcox <matthew@....cx>
Cc:	Hisashi Hifumi <hifumi.hisashi@....ntt.co.jp>,
	akpm@...ux-foundation.org, linux-ext4@...r.kernel.org,
	linux-fsdevel@...r.kernel.org
Subject: Re: [RESEND] [PATCH] lseek: change i_mutex usage.

On Thu, Jan 15, 2009 at 06:22:52AM -0700, Matthew Wilcox wrote:
> 
> Of course if you have multiple threads, they will share a struct file,
> and you're updating f_pos and f_version without locking.  Maybe that's
> OK, but it's soemthing you didn't discuss.

f_pos is updated by sys_write(), and friends without locking, so we're
fine on that front, or at least no worse off.  SUSv3 doesn't seem to
say one way or another what should happen if two threads try to
write() to a file at the same time using the same file descriptor in
terms of whether or not f_pos gets updated intelligently.  We've opted
for speed over determinism already.

Zero'ing out f_version is fine to do without locking.  It's only used
so we know that we need to revalidate in the readdir() case so that we
know it's pointing at a valid directory pointer.

That being said, I do see a race in fs/ext*/dir.c, but i_mutex locking
isn't the problem and it's not going to save us.  ext[234]_readdir()
uses f_pos through the routine, even between calls that might block;
so if one thread is randomly calling seekdir() (or lseek() directly)
while another read is calling readdir(), ext[234]_readdir() could get
potentially very confused.  If someone wants to take a look at it,
that would be great.  Otherwise I'll put it on my low-priority queue
of things to look at.

> I think it's the only reason to have the mutex here.  Otherwise we could
> simply use i_size_read() in generic_file_llseek_unlocked() and there
> would be no need for a mutex at all.

That's a good point.  Maybe I'm missing something, but I'm not sure we
need the mutex in generic_file_llseek() at all.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html