droyo
commented
6 years ago Thank you for your report. If you do happen to come across the code that triggered this issue, please let me know.
Here's what read(5) from the plan9 manual has to say about directories:
For directories, read returns an integral number of direc-
tory entries exactly as in stat (see stat(5)), one for each
member of the directory. The read request message must have
offset equal to zero or the value of offset in the previous
read on the directory, plus the number of bytes returned in
the previous read. In other words, seeking other than to
the beginning is illegal in a directory (see seek(2)).I'm not sure what "integral" means here. I had taken it to mean read doesn't return a partial directory entry.
The problem that this portion of the code deals with is what to do when Tread does not request enough bytes to contain the largest possible directory entry. There are two approaches here:
- Return zero bytes. Conventionally in 9P this is used to signal EOF.
- Write part of the stat to the buffer. The client (v9fs, 9pfuse) will then request the rest in another Tread call.
What your change does is to leave nstats at 0, and inadvertently read the whole directory (Readdir(0) lists all fileinfos). The reason this worked for you is because the maximum size of a Stat is larger than any common file (it allows for a very large user, group, and file name), so in practice it worked out. However, it is incorrect, because if you do run into a file with a huge file/owner name, you'll get an error marshalling the FileInfo into the buffer.
I think the only choice here is to partially write the Stat message, and keep the unwritten part in a buffer for furture reads.
When attempting to send 9p messages larger than the 9p message size agreed upon by the client and server, some clients - e.g., the Linux 9p driver - will perform 'partial reads' of messages, breaking things up into small enough pieces to work. This means that while a message may end up being larger than the agreed-upon size, those messages can still successfully be transferred.
In this patch, I removed the line of code that was artificially preventing those connections from working - a change I had made in order to get a personal project up and running. This meant that the code went from messages that were too large not being sent, to messages that were too large successfully transferring. My client was the standard linux 9p module.
Unfortunately, I was awful and didn't save the code that was running into the issue for a reproducible - but I thought this PR might still help track down how you'd like to approach the problem.
Thanks!
Marzhall