openssh bugs - May 2003 - [Bug 581] SFTP "ls" listings never end

http://bugzilla.mindrot.org/show_bug.cgi?id=581

           Summary: SFTP "ls" listings never end
           Product: Portable OpenSSH
           Version: -current
          Platform: Alpha
        OS/Version: OSF/1
            Status: NEW
          Severity: major
          Priority: P2
         Component: sftp-server
        AssignedTo: openssh-bugs at mindrot.org
        ReportedBy: from-bugzilla at geek-central.gen.nz


When the OpenSSH SFTP server running on Compaq/HP Tru64 UNIX 5.1 
is sent an "ls" command for a directory on an NFS-mounted volume, it 
either returns an endlessly-repeating directory listing, or seems to hang 
without returning anything.

The problem has to do with the semantics of the readdir call, as used in 
the process_readdir routine in sftp-server.c. readdir returns NULL to 
indicate the end of the directory listing. However, process_readdir may 
then later call readdir again on the same directory handle. For a local 
volume, it will continue returning NULL. However, for an NFS-mounted 
volume, after returning NULL once, on Tru64 it will start returning the 
entire directory listing again.

Solution: add a separate Boolean flag to the Handle structure. This flag is 
initialized to false when a new directory context is created (it is not needed 
for file contexts). It is set to true when a readdir call returns NULL. Once it 
is true, no subsequent calls to readdir are to be made on that context; 
instead the next call to process_readdir will immediately return a 
SSH2_FX_EOF status.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From djm at mindrot.org  2003-06-01 00:53 -------
How is this a bug in sftp and not a bug in Tru64?



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From mouring at eviladmin.org  2003-06-01 05:11
-------
What client?

As far as I can see your solution would not work.  Since we open a FD handle 
on the server, process on that handle, then close/remove that handle.

The only non-freed case is if the handle passed to it is not a HANDLE_DIR or 
HANDLE_FILE or the handle is invalid.  Which then returns an errno of ENOENT.

So unless your hitting some odd memory corruption.  I'm not seeing how you
are
getting this behavior unless the client your using is not RFC legal.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From dtucker at zip.com.au  2003-06-03 23:27 -------
Err, if this bug is innapropriate, blame me.  I suggested that if you 
squinted and read the posix spec [1] at an angle in bad light the behaviour of 
Tru64's readdir might not be non-compliant.

It says it must return "an ordered sequence of all the directory
entries" which
it apparently does, and return a NULL at the end, which it apparently does.

Ben's point about the client is a good one though.  What is it?

[1] http://www.opengroup.org/onlinepubs/007904975/functions/readdir.html



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From from-bugzilla at geek-central.gen.nz 
2003-06-04 07:24 -------
Created an attachment (id=319)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=319&action=view)
Fixes the problem

The client used was OpenSSH 3.6.1ps SFTP, though I expect the problem would
happen with any client.

The bug is triggered because each time process_readdir is called, it collects
up to 100 entries from readdir, or until readdir returns NULL. If it collected
any entries at all, it doesn't remember which case it encountered. So it
will
get called again to return more entries, so it calls readdir again, which can
start returning the entire directory listing again. And so on, and so on.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From djm at mindrot.org  2003-06-04 08:19 -------
Could you please attach that in unified "diff -u" format? These are
much easier
to read.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From from-bugzilla at geek-central.gen.nz 
2003-06-04 08:44 -------
Sorry, no -u option on Tru64 diff:

diff -u sftp-server.c*
diff: illegal option -- u
Usage: diff [-bitw] [-c |-C Lines|-D String|-e|-f|-h|-n] File1 File2
       diff [-bilrstw] [-c |-C Lines|-e|-f|-h|-n] [-S File] Directory1
Directory2




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From djm at mindrot.org  2003-06-04 09:05 -------
Try diff -c then



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From mouring at eviladmin.org  2003-06-04 09:22
-------
There are two aspects to this bug.

1. Tru64's kernel is broken. =)  After readdir() returns NULL the first time
it should return NULL every other time.  Something Tru64 does not do when it 
is access files via NFS.

2. sftp-server depends on this behavior.

the Posix definition just states "the end of the directory is encountered,
a
null pointer shall be returned and errno is not changed."  which to me
implies
it should not rewind() the DIR* structure.  Which is what seems to be 
happening.

You need to really report this bug to Compaq/HP.  Before I consider any work 
arounds I would perfer to know that this has not been resolved in a service 
pack.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From from-bugzilla at geek-central.gen.nz 
2003-06-04 17:11 -------
Created an attachment (id=320)
 --> (http://bugzilla.mindrot.org/attachment.cgi?id=320&action=view)
Fixes the problem

OK, here's a diff -u done from a Linux box.



------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From from-bugzilla at geek-central.gen.nz 
2003-06-04 17:31 -------
Which way is Tru64's kernel "broken":

1) It doesn't do it the way Linux does (true enough), or
2) It doesn't conform to the official spec (not so clear).

Darren Tucker has previously referenced the spec at <http://
www.opengroup.org/onlinepubs/007904975/functions/readdir.html>. The 
relevant paragraph is the following:

The readdir() function shall return a pointer to a structure representing the 
directory entry at the current position in the directory stream specified by 
the argument dirp, and position the directory stream at the next entry. It 
shall return a null pointer upon reaching the end of the directory stream. 
The structure dirent defined in the <dirent.h> header describes a
directory
entry.

Now, what does this spec say about what happens when you call readdir 
again _after_ it has returned a null pointer? It's not clear at all. There 
doesn't seem to be any prerequisite attached to the post-condition "
...and
position the directory stream at the next entry". So what is the
"next" entry
after the end of the directory stream? The Tru64 interpretation (start over 
from the beginning) seems as reasonable as anything else.

Remember the robustness principle <ftp://ftp.isi.edu/in-notes/rfc791.txt>:
"be liberal in what you accept, conservative in what you send."




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

http://bugzilla.mindrot.org/show_bug.cgi?id=581





------- Additional Comments From djm at mindrot.org  2003-06-04 17:58 -------
Have you checked whether this is a reported bug with DEC/Compaq/HP?

This *is* a bug - Neither BSD, Linux, Sun nor even Tru64 for non-NFS mounts
behave this way. While SUSv3 and Darren's reference don't mention what
happens
after the initial NULL is returned, the behaviour to rewinddir() back to the
beginning of the directory is so utterly unexpected and bogus that it should not
need to be specified against.

I'd consider including a workaround if you can point to system documentation
describing this behaviour as being intended or if DEC/Compaq/HP don't accept
this as a bug.




------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.