Message ID | 20140430091330.GV10922@spoyarek.pnq.redhat.com |
---|---|
State | New |
Headers | show |
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.
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 {