Tom, thanks for review. -jan
On 20/04/10 14:21 -0700, Tom Haynes wrote: > 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]
