Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Dec 2003 21:43:50 -0600
From:      "Jacques A. Vidrine" <nectar@FreeBSD.org>
To:        Lachlan O'Dea <odela01@ca.com>
Cc:        freebsd-bugs@FreeBSD.org
Subject:   Re: bin/60287: [patch] NSS does not handle NSS_STATUS_TRYAGAIN properly
Message-ID:  <20031219034350.GA59636@hellblazer.celabo.org>
In-Reply-To: <778706BE-31D1-11D8-867A-000A95DBB47C@ca.com>
References:  <200312181414.hBIEENcW093868@freefall.freebsd.org> <20031218145453.GB35590@madman.celabo.org> <778706BE-31D1-11D8-867A-000A95DBB47C@ca.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Dec 19, 2003 at 02:14:33PM +1100, Lachlan O'Dea wrote:
> Are you certain that the module should actually set errno? At the 
> moment, nss_ldap does return ERANGE, but not in errno.

Well, NSS modules do not set errno directly but rather use the supplied
errnop argument.  But yes, it must set errno.

> In the LDAP 
> case, getgrent seems to end up calling:
> 
> NSS_STATUS
> _nss_ldap_getgrent_r (struct group *result,
>                       char *buffer, size_t buflen, int *errnop)
> 
> In the buffer too small case, *errnop _is_ set to ERANGE. 

Not that I saw... I was looking for what nss_ldap does after checking
`bytesleft', e.g. from _nss_ldap_parse_gr:

      if (bytesleft (buffer, buflen, char *) <
          (dn_mems_c + 1) * sizeof (char *))
        {
          ldap_value_free (vals);
          return NSS_TRYAGAIN;
        }

I believe it should do `*errnop = ERANGE' in this case.

> This ends up 
> being checked by libc before it expands the buffer in getgr in 
> getgrent.c.
> 
> In any case, I don't think my patch needs to be changed? I think you're 
> saying that the errno code tells us why TRYAGAIN was returned. But 
> regardless of why, TRYAGAIN implies that the operation can be, well, 
> tried again. So we never want to set the terminate flag in the TRYAGAIN 
> case, regardless of the errno value.

No.  The behavior of `TRYAGAIN' depends upon what is set in
/etc/nsswitch.conf, and has nothing whatsoever to do with the
FreeBSD-specific implementation of getgrent which is layered on top
of getgrent_r.  The actual fix is multifold:

  1. the GNU libc compat stuff should not set the terminator in
     the NS_RETURN case
  2. the GNU libc compat stuff should `translate' the condition
     `result == NSS_STATUS_TRYAGAIN && *errnop == ERANGE' into
     `result == NS_RETURN && *errnop == ERANGE'
  3. nss_ldap should set ERANGE when it is given a buffer that is
     too small, as required by the GNU libc documentation 

I'll be happy to discuss further when I return!  Cheers,
-- 
Jacques Vidrine   . NTT/Verio SME      . FreeBSD UNIX       . Heimdal
nectar@celabo.org . jvidrine@verio.net . nectar@freebsd.org . nectar@kth.se



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20031219034350.GA59636>