still nfs problems [Was: Linux 2.6.37-rc8]

[Linus Torvalds]

On Thu, Dec 30, 2010 at 11:25 AM, Trond Myklebust
<Trond.Myklebust at netapp.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