xdr: Avoid xdr_string maxsize check when freeing

The maxsize argument in xdr_string() is garbage when called by
xdr_free(), since xdr_free() only passes the XDR handle and the xdr
string to be freed. Sometimes the size check fails and xdr_string()
returns early, without freeing the string and without setting the object
pointer to NULL.

Usually this just results in leaking the string's memory. But since
commit 9ae5b599c7 (bos: Let xdr allocate rpc output strings), many
callers in bos.c rely on xdr_free(xdr_string) to set the given string
to NULL; if this doesn't happen, subsequent calls to BOZO_ RPCs can
corrupt memory, often causing the 'bos' process to segfault.

We only need the maxsize check when encoding or decoding, so avoid
accessing the maxsize agument when the op mode is XDR_FREE.

In general, xdr_free() can only safely be used on xdr 2-argument xdr
functions, so must be avoided when freeing xdr opaque, byte, and union
types.

This change makes it safe to use xdr_free() to free xdr strings, but in
the future, we should provide a typesafe and less fragile function for
freeing xdr strings returned from RPCs.  Currently, xdr_free(xdr_string)
is only called by the bos client and the tests.

Change-Id: I2bbd6cfe48b77f1fd826207b69f50df9c0158a10
Reviewed-on: https://gerrit.openafs.org/15343
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

diff --git a/src/rx/xdr.c b/src/rx/xdr.c
index bfc1b7f..88f7772 100644 (file)
--- a/src/rx/xdr.c
+++ b/src/rx/xdr.c
@@ -530,7 +530,7 @@ xdr_string(XDR * xdrs, char **cpp, u_int maxsize)
     if (!xdr_u_int(xdrs, &size)) {
        return (FALSE);
     }
-    if (size > maxsize) {
+    if (xdrs->x_op != XDR_FREE && size > maxsize) {
        return (FALSE);
     }
     nodesize = size + 1;