OpenSolaris

Printable Version Enter a New Search
Bug ID 4023944
Synopsis rpcgen generates linked list routines that blow stacks
State 10-Fix Delivered (Fix available in build)
Category:Subcategory network:rpc
Keywords automountd | mountd | overflow | rpcgen | s10-reviewed | stack | sunalert-submit-availability
Responsible Engineer Bill Ricker
Reported Against 5.8 , s9u2 , s10_37 , s297_beta , s297hw2_03
Duplicate Of
Introduced In sunos_4.1
Commit to Fix snv_14
Fixed In snv_14
Release Fixed solaris_nevada(snv_14)
Related Bugs 1197979 , 4023817 , 4071426 , 4082402 , 4082403 , 4613875 , 4886458
Submit Date 3-January-1997
Last Update Date 17-January-2006
Description
brent.callaghan@Eng 1997-01-03

There are two bugs now filed (1197979 and 4023817) that describe
situations where the automounter and the mount daemon crashed
through stack overflow from excessive recursion.

The recursion resulted from code generated by rpcgen for
encode/decode of linked lists.  The code seems to be
optimized for the case of arbitrary trees where recursion
would be more suitable - rather than the trivial case of
a linked list where an iterative technique would be better.
An intelligent fix might even have rpcgen produce iterative
code in the general case.

For example rpcgen translates the following:

	struct entry {
	        int     foo;
	        struct entry *next;
	};

into the XDR routine:

        bool_t
        xdr_entry(xdrs, objp)
                register XDR *xdrs;
                entry *objp;
        {
                if (!xdr_int(xdrs, &objp->foo))
                        return (FALSE);
                if (!xdr_pointer(xdrs, (char **)&objp->next, 
				sizeof (entry), (xdrproc_t) xdr_entry))
                        return (FALSE);
                return (TRUE);
        }

Note the use of recursion on xdr_pointer -> xdr_entry ->x dr_pointer-> ...

The XDR routine would be more efficient and less dangerous
using tail recursion:

        bool_t
        xdr_entry(xdrs, objp)
                register XDR *xdrs;
                entry *objp;
        {
                if (!xdr_int(xdrs, &objp->foo))
                        return (FALSE);
 
                return (xdr_pointer(xdrs, (char **)&objp->next,
				sizeof (entry), (xdrproc_t) xdr_entry));
        }

or a purely iterative technique:

        bool_t
        xdr_entry(xdrs, objp)
                register XDR *xdrs;
                entry *objp;
        {
                bool_t more_data;

                for(;;) {

                        more_data = objp != NULL;
                        if (!xdr_bool(xdrs, &more_data))
                                return (FALSE);
                 
                        if (!more_data)
                                break;
                 
                        if (!xdr_int(xdrs, &objp->foo))
                                return (FALSE);
                 
                        objp = objp->next;
                }

                return (TRUE);
        }

Which has no stack implications and is likely faster to boot.

The fix for mountd (bug 4023817) was to build an iterative version
of xdr_mountlist() that replaces the version generated by rpcgen
in librpcsvc.  However, this replacement routine is private to mountd.
This is a bit sad also for mountd - since prior to the bugfix, all the
MOUNT protocol XDR routines were derived directly from the mount.x
file thanks to rpcgen.  



See Attached for new suggested fix.

 xxxxx@xxxxx.com 2005-04-04 19:27:29 GMT
Work Around
N/A
Comments
N/A