rxgen: Handle complex structures
authorSimon Wilkinson <sxw@inf.ed.ac.uk>
Wed, 1 Sep 2010 13:38:58 +0000 (14:38 +0100)
committerDerrick Brashear <shadow@dementia.org>
Tue, 21 Sep 2010 12:12:11 +0000 (05:12 -0700)
Servers built using rxgen will break if they take complex
structures as RPC arguments. A complex structure, in this case, is
one which contains an array.

For example an RPC which takes as an argument:

struct MyData {
    opaque somebytes<>;
}

... will cause memory corruption on the server whenever it is called.

This is becase the server stubs emitted by rxgen do not zero out the
contents of the MyData structure, leaving it with whatever garbage may
be on the stack. When XDR comes to populate the somebytes opaque
array, it sees that MyData.somebytes.somebytes_val is non-zero, and
assumes that this is a pre-allocated block into which it can record
the data from the wire. However, it's really just stack garbage, and
so we overwrite memory.

As a fix, this patch creates a new list of 'complex' structures, which
are identified as structures which contain arrays. When a server stub
is created for a function that takes a complex structure, the structure
is set to zero before use, and marked to be freed afterwards.

I suspect that there may be a wider class of complex structures than are
caught by this routine, but this is a start...

Change-Id: Id671fe602c8cd44afaaccc821aaa097b142f1899
Reviewed-on: http://gerrit.openafs.org/2736
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

src/rxgen/rpc_parse.c

index 28f07fa..da0dd32 100644 (file)
@@ -45,7 +45,7 @@
 #include "rpc_util.h"
 
 list *proc_defined[MAX_PACKAGES], *special_defined, *typedef_defined,
-    *uniondef_defined;
+    *uniondef_defined, *complex_defined;
 char *SplitStart = NULL;
 char *SplitEnd = NULL;
 char *MasterPrefix = NULL;
@@ -249,6 +249,7 @@ def_struct(definition * defp)
     declaration dec;
     decl_list *decls;
     decl_list **tailp;
+    int special = 0;
 
     defp->def_kind = DEF_STRUCT;
 
@@ -258,6 +259,11 @@ def_struct(definition * defp)
     tailp = &defp->def.st.decls;
     do {
        get_declaration(&dec, DEF_STRUCT);
+       /* If a structure contains an array, then we're going
+        * to need to be clever about freeing it */
+       if (dec.rel == REL_ARRAY) {
+          special = 1;
+       }
        decls = ALLOC(decl_list);
        decls->decl = dec;
        *tailp = decls;
@@ -267,6 +273,9 @@ def_struct(definition * defp)
     } while (tok.kind != TOK_RBRACE);
     get_token(&tok);
     *tailp = NULL;
+
+    if (special)
+       STOREVAL(&complex_defined, defp);
 }
 
 static void
@@ -1487,6 +1496,7 @@ ss_ProcSpecial_setup(definition * defp, int *somefrees)
 
     for (listp = special_defined; listp != NULL; listp = listp->next) {
        defp1 = (definition *) listp->val;
+
        for (plist = defp->pc.plists; plist; plist = plist->next) {
            if (plist->component_kind == DEF_PARAM
                && (plist->pl.param_kind == DEF_INPARAM
@@ -1534,6 +1544,20 @@ ss_ProcSpecial_setup(definition * defp, int *somefrees)
            }
        }
     }
+    for (listp = complex_defined; listp != NULL; listp = listp->next) {
+       defp1 = (definition *) listp->val;
+       for (plist = defp->pc.plists; plist; plist = plist->next) {
+           if (plist->component_kind == DEF_PARAM) {
+               if (streq(defp1->def_name, structname(plist->pl.param_type))) {
+                   plist->pl.param_flag |= FREETHIS_PARAM;
+                   *somefrees = 1;
+                   fprintf(fout, "\n\tmemset(&%s, 0, sizeof(%s));",
+                                plist->pl.param_name, defp1->def_name);
+               }
+           }
+       }
+    }
+
     f_print(fout, "\n");
 }