linux-kernel - Re: still nfs problems [Was: Linux 2.6.37-rc8]

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, 30 Dec 2010 12:02:23 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Trond Myklebust <Trond.Myklebust@...app.com>
Cc:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>,
	Chuck Lever <chuck.lever@...cle.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Arnd Bergmann <arnd@...db.de>, linux-nfs@...r.kernel.org
Subject: Re: still nfs problems [Was: Linux 2.6.37-rc8]

On Thu, Dec 30, 2010 at 11:25 AM, Trond Myklebust
<Trond.Myklebust@...app.com> wrote:
>
> uncached_readdir is not really a problem. The real problem is
> filesystems that generate "infinite directories" by producing looping
> combinations of cookies.

But if we don't have any lseek's, the readdir cache should trivially
take care of this by just incrementing the page_index, and we should
return to user space the (eventually ending) sequence, even if there
are duplicate numbers.

(Also, I suspect that "page_index" should not be a page index, but a
position, and then you the "search_for_pos()" should use that instead
of the file_pos/current_index thing, but that's a detail that would
show up only when you have duplicate cookies within one page worth of
directory caches)

And if the server really sends us an infinite stream of entries, then
that's fine - at least we give to user space the infinite entries that
were given to us, instead of _generating_ an infinite stream from what
was a finite - but broken - stream).

So it seems wrong that the directory caching code resets page_index to
the start when it then does an uncached readdir. That seems wrong.

I'm sure there's some reason for it, but wouldn't it be nice if the
rule for page_index was that it starts off at zero, and only gets
reset by lseek?

> IOW: I've seen servers that generate cookies in a sequence of a form
> vaguely resembling
>
> 1 2 3 4 5 6 7 8 9 10 11 12 3...
>
> (with possibly a thousand or so entries between the first and second
> copy of '3')

Ok, so that' obviously broken, but it's then _doubly_ broken to turn
that long broken sequence into an _endless_ broken sequence.

And I agree that when user space sees such an endless broken sequence,
it's a real stopping problem for user space. But in the absense of
lseek, it should _never_ be a problem for the kernel itself, afaik.
The kernel should happily return just the broken sequence. No?

So then perhaps the solution is to just remove the resetting of
page_index in the uncached_readdir() function? Make sure that the
page_index is monotonically increasing for any readdir(), and you
protect against turning a bad sequence into an endless sequence.

Of course, lseek() will have to reset page_index to zero, and if
somebody does an lseek() on the directory, then the duplicate '3"
entry in the cookie sequence will inevitably be ambiguous, but that
really is unavoidable. And rare. People who use lseek() on directories
are odd and we know they'll break on other filesystems too under
certain circumstances.

                         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/