We've run into a couple of scenarios lately where a broken metadata file
can cause the server to crash. The latest one looks like this:
------------------
- a metadata file exists, but the db entry for its datafile handles (the
"dh" key is missing for some reason
- a client requests a getattr on the file
- the server reads the basic attributes successfully, indicating that
there is a dfile array of size x and a distribution of size y
- the server tries to read the dfile array but fails
- at this point the dfile array pointer in the attributes structure is
non-zero but has garbage in it
- at this point the distribution pointer in the attributes structure is NULL
- the server state machine follows a failure path to send a response
with an error status
- the encoder tries to encode the response, but segfaults because it
thinks the distribution exists (the mask is set and the size is set to
y) when really it is a NULL pointer
------------------
We've plugged some similar bugs before by just fixing the specific case
(being more careful about cleaning up masks, pointers, sizes, etc. after
an error), and the same could easily be done here. Should there be a
more general solution, though?
What I am wondering is if the response encoder should even bother to
encode the whole message if the status is non-zero. It seems like if
there is an error code it should just stick to encoding the basic
PVFS_server_resp struct. The rest of the fields can't really be trusted
since we hit an error condition. Likewise on the decode side of things.
The lebf_encode_resp() function already sort of catches one case of this:
/* we stand a good chance of segfaulting if we try to encode the
response
* after something bad happened reading data from disk. */
if (resp->status != -PVFS_EIO)
{
... but that only handles one specific error code.
I don't think it would be a real big change to do this in a more general
manner. The biggest problem is that it throws off the size checking
logic that checks buffer sizes after encoding, but that shouldn't be
hard to adjust.
Any thoughts? Is this the right approach, or should we continue to
encode/decode all of the request-specific regardless of error?
-Phil
_______________________________________________
Pvfs2-developers mailing list
[email protected]
http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
