Use NSS_STATUS_TRYAGAIN to indicate insufficient buffer (BZ #16878)
Commit Message
The netgroups nss modules in the glibc tree use NSS_STATUS_UNAVAIL
(with errno as ERANGE) when the supplied buffer does not have
sufficient space for the result. This is wrong, because the canonical
way to indicate insufficient buffer is to set the errno to ERANGE and
the status to NSS_STATUS_TRYAGAIN, as is used by all other modules.
This fixes nscd behaviour when the nss_ldap module returns
NSS_STATUS_TRYAGAIN to indicate that a netgroup entry is too long to
fit into the supplied buffer. A reproducer is present on the bz
#16878, which I used to verify that the problem is fixed. Many thanks
to Michael Weiser for the reproducer and analysis.
Siddhesh
[BZ #16878]
* nscd/netgroupcache.c (addgetnetgrentX): Look for
NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
* nscd/nss_files/files-netgrp.c (_nss_netgroup_parseline): Use
NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
---
nscd/netgroupcache.c | 14 ++++++++------
nss/nss_files/files-netgrp.c | 2 +-
2 files changed, 9 insertions(+), 7 deletions(-)
Comments
Ping!
On Wed, Apr 30, 2014 at 02:43:30PM +0530, Siddhesh Poyarekar wrote:
> The netgroups nss modules in the glibc tree use NSS_STATUS_UNAVAIL
> (with errno as ERANGE) when the supplied buffer does not have
> sufficient space for the result. This is wrong, because the canonical
> way to indicate insufficient buffer is to set the errno to ERANGE and
> the status to NSS_STATUS_TRYAGAIN, as is used by all other modules.
>
> This fixes nscd behaviour when the nss_ldap module returns
> NSS_STATUS_TRYAGAIN to indicate that a netgroup entry is too long to
> fit into the supplied buffer. A reproducer is present on the bz
> #16878, which I used to verify that the problem is fixed. Many thanks
> to Michael Weiser for the reproducer and analysis.
>
> Siddhesh
>
> [BZ #16878]
> * nscd/netgroupcache.c (addgetnetgrentX): Look for
> NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
> * nscd/nss_files/files-netgrp.c (_nss_netgroup_parseline): Use
> NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
>
> ---
> nscd/netgroupcache.c | 14 ++++++++------
> nss/nss_files/files-netgrp.c | 2 +-
> 2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> index b3d40e9..edab174 100644
> --- a/nscd/netgroupcache.c
> +++ b/nscd/netgroupcache.c
> @@ -197,11 +197,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
> int e;
> status = getfct.f (&data, buffer + buffilled,
> buflen - buffilled - req->key_len, &e);
> - if (status == NSS_STATUS_RETURN
> - || status == NSS_STATUS_NOTFOUND)
> - /* This was either the last one for this group or the
> - group was empty. Look at next group if available. */
> - break;
> if (status == NSS_STATUS_SUCCESS)
> {
> if (data.type == triple_val)
> @@ -320,11 +315,18 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
> }
> }
> }
> - else if (status == NSS_STATUS_UNAVAIL && e == ERANGE)
> + else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE)
> {
> buflen *= 2;
> buffer = xrealloc (buffer, buflen);
> }
> + else if (status == NSS_STATUS_RETURN
> + || status == NSS_STATUS_NOTFOUND
> + || status == NSS_STATUS_UNAVAIL)
> + /* This was either the last one for this group or the
> + group was empty or the NSS module had an internal
> + failure. Look at next group if available. */
> + break;
> }
>
> enum nss_status (*endfct) (struct __netgrent *);
> diff --git a/nss/nss_files/files-netgrp.c b/nss/nss_files/files-netgrp.c
> index 34eae4c..bc0b367 100644
> --- a/nss/nss_files/files-netgrp.c
> +++ b/nss/nss_files/files-netgrp.c
> @@ -252,7 +252,7 @@ _nss_netgroup_parseline (char **cursor, struct __netgrent *result,
> if (cp - host > buflen)
> {
> *errnop = ERANGE;
> - status = NSS_STATUS_UNAVAIL;
> + status = NSS_STATUS_TRYAGAIN;
> }
> else
> {
> --
> 1.8.3.1
>
Ping!
On Thu, May 15, 2014 at 02:12:32PM +0530, Siddhesh Poyarekar wrote:
> Ping!
>
> On Wed, Apr 30, 2014 at 02:43:30PM +0530, Siddhesh Poyarekar wrote:
> > The netgroups nss modules in the glibc tree use NSS_STATUS_UNAVAIL
> > (with errno as ERANGE) when the supplied buffer does not have
> > sufficient space for the result. This is wrong, because the canonical
> > way to indicate insufficient buffer is to set the errno to ERANGE and
> > the status to NSS_STATUS_TRYAGAIN, as is used by all other modules.
> >
> > This fixes nscd behaviour when the nss_ldap module returns
> > NSS_STATUS_TRYAGAIN to indicate that a netgroup entry is too long to
> > fit into the supplied buffer. A reproducer is present on the bz
> > #16878, which I used to verify that the problem is fixed. Many thanks
> > to Michael Weiser for the reproducer and analysis.
> >
> > Siddhesh
> >
> > [BZ #16878]
> > * nscd/netgroupcache.c (addgetnetgrentX): Look for
> > NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
> > * nscd/nss_files/files-netgrp.c (_nss_netgroup_parseline): Use
> > NSS_STATUS_TRYAGAIN to indicate insufficient buffer space.
> >
> > ---
> > nscd/netgroupcache.c | 14 ++++++++------
> > nss/nss_files/files-netgrp.c | 2 +-
> > 2 files changed, 9 insertions(+), 7 deletions(-)
> >
> > diff --git a/nscd/netgroupcache.c b/nscd/netgroupcache.c
> > index b3d40e9..edab174 100644
> > --- a/nscd/netgroupcache.c
> > +++ b/nscd/netgroupcache.c
> > @@ -197,11 +197,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
> > int e;
> > status = getfct.f (&data, buffer + buffilled,
> > buflen - buffilled - req->key_len, &e);
> > - if (status == NSS_STATUS_RETURN
> > - || status == NSS_STATUS_NOTFOUND)
> > - /* This was either the last one for this group or the
> > - group was empty. Look at next group if available. */
> > - break;
> > if (status == NSS_STATUS_SUCCESS)
> > {
> > if (data.type == triple_val)
> > @@ -320,11 +315,18 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
> > }
> > }
> > }
> > - else if (status == NSS_STATUS_UNAVAIL && e == ERANGE)
> > + else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE)
> > {
> > buflen *= 2;
> > buffer = xrealloc (buffer, buflen);
> > }
> > + else if (status == NSS_STATUS_RETURN
> > + || status == NSS_STATUS_NOTFOUND
> > + || status == NSS_STATUS_UNAVAIL)
> > + /* This was either the last one for this group or the
> > + group was empty or the NSS module had an internal
> > + failure. Look at next group if available. */
> > + break;
> > }
> >
> > enum nss_status (*endfct) (struct __netgrent *);
> > diff --git a/nss/nss_files/files-netgrp.c b/nss/nss_files/files-netgrp.c
> > index 34eae4c..bc0b367 100644
> > --- a/nss/nss_files/files-netgrp.c
> > +++ b/nss/nss_files/files-netgrp.c
> > @@ -252,7 +252,7 @@ _nss_netgroup_parseline (char **cursor, struct __netgrent *result,
> > if (cp - host > buflen)
> > {
> > *errnop = ERANGE;
> > - status = NSS_STATUS_UNAVAIL;
> > + status = NSS_STATUS_TRYAGAIN;
> > }
> > else
> > {
> > --
> > 1.8.3.1
> >
>
>
On Thu, May 15, 2014 at 02:12:32PM +0530, Siddhesh Poyarekar wrote:
> Ping!
>
> On Wed, Apr 30, 2014 at 02:43:30PM +0530, Siddhesh Poyarekar wrote:
> > The netgroups nss modules in the glibc tree use NSS_STATUS_UNAVAIL
> > (with errno as ERANGE) when the supplied buffer does not have
> > sufficient space for the result. This is wrong, because the canonical
> > way to indicate insufficient buffer is to set the errno to ERANGE and
> > the status to NSS_STATUS_TRYAGAIN, as is used by all other modules.
> >
> > This fixes nscd behaviour when the nss_ldap module returns
> > NSS_STATUS_TRYAGAIN to indicate that a netgroup entry is too long to
> > fit into the supplied buffer. A reproducer is present on the bz
> > #16878, which I used to verify that the problem is fixed. Many thanks
> > to Michael Weiser for the reproducer and analysis.
> >
looks ok.
@@ -197,11 +197,6 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
int e;
status = getfct.f (&data, buffer + buffilled,
buflen - buffilled - req->key_len, &e);
- if (status == NSS_STATUS_RETURN
- || status == NSS_STATUS_NOTFOUND)
- /* This was either the last one for this group or the
- group was empty. Look at next group if available. */
- break;
if (status == NSS_STATUS_SUCCESS)
{
if (data.type == triple_val)
@@ -320,11 +315,18 @@ addgetnetgrentX (struct database_dyn *db, int fd, request_header *req,
}
}
}
- else if (status == NSS_STATUS_UNAVAIL && e == ERANGE)
+ else if (status == NSS_STATUS_TRYAGAIN && e == ERANGE)
{
buflen *= 2;
buffer = xrealloc (buffer, buflen);
}
+ else if (status == NSS_STATUS_RETURN
+ || status == NSS_STATUS_NOTFOUND
+ || status == NSS_STATUS_UNAVAIL)
+ /* This was either the last one for this group or the
+ group was empty or the NSS module had an internal
+ failure. Look at next group if available. */
+ break;
}
enum nss_status (*endfct) (struct __netgrent *);
@@ -252,7 +252,7 @@ _nss_netgroup_parseline (char **cursor, struct __netgrent *result,
if (cp - host > buflen)
{
*errnop = ERANGE;
- status = NSS_STATUS_UNAVAIL;
+ status = NSS_STATUS_TRYAGAIN;
}
else
{