On 04/20/10 02:52 PM, Jan Kryl wrote: > Hi, > > could you review the fix for 6923303 (mountd dumped core on > server ...)? >
The changes look good. I agree the the code looks more legible with your changes. > webrev: http://cr.opensolaris.org/~jkryl/nfs-accesslist-1/ > > thanks > -jan > > > Background information: > > On behalf of CR 6882460 was introduced lazy lookup of client's hostname, > which means that netbuf and nd_hostservlist aren't populated until we need > to use them. However there is a mistake in in_access_list(): > > 1681 if (*pnb == NULL) { > 1682 lookup_names = TRUE; > 1683 *pnb = svc_getrpccaller(transp); > 1684 if (*pnb == NULL) > 1685 *pclnames = anon_client(NULL); > 1686 } else > 1687 lookup_names = FALSE; > > If pnb is not NULL we populate pnb and pclnames remains NULL. If pclnames > isn't used afterwards in in_access_list(), then when we enter in_access_list() > second time, pnb isn't NULL, so we don't set lookup_names to TRUE. However > we access pclnames in for loop at line 1739, which results in NULL pointer > dereference. > > Another problem of current implementation is that return value from > anon_client() isn't checked for being NULL. Instead "serv" output parameter > of getclientsnames() is tested for NULL. getclientsnames() and friends > should check return value of anon_client() and return error, if malloc() > in anon_client() failed. This would make the current code more readable > and less error-prone. > _______________________________________________ > nfs-discuss mailing list > [email protected] > _______________________________________________ nfs-discuss mailing list [email protected]
