#3589 nss: Fix invalid enum nss_status return values.
Closed by lslebodn. Opened by codonell.
Unknown source master

Download 3589.patch

The upstream glibc test nss/bug17079 covers several cases where the
NSS infrastructure passes invalid pointers to NSS plugins. The plugins
should return correct results for the invalid values e.g. ERANGE,
but it should do so by setting errnop to the error and returning
NSS_STATUS_TRYAGAIN. This commit fixes the group, netgroup, passwd
and service handling code to correctly return ERANGE in
errnop
and NSS_TATUS_TRYAGAIN in the case of invalid buffer (NULL) or
zero sized buffer length. This fixes the nss/bug17079 regression test
when run in a test configuration with sss enabled for any of the
above mentioned services.

Upstream glibc bug:
Bug 22530 - FAIL: nss/bug17079 due to _nss_sss_getpwuid_r
https://sourceware.org/bugzilla/show_bug.cgi?id=22530

Signed-off-by: Carlos O'Donell carlos@redhat.com

This doesn't look right:

 5 -     if (!buffer || !buflen) return ERANGE;
 6 +     if (!buffer || !buflen) {
 7 + »       *errnop = ERANGE;
 8 + »       return NSS_STATUS_SUCCESS;
 9 +     }

It should be NSS_STATUS_TRYAGAIN.

Thank you guys for patch.

Could you also check other parts in the same file.
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_734
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_749
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_769

NSS_STATUS_NOTFOUND is returned only with enabled switch --enable-sss-default-nss-plugin. It is enabled in fedora and el7 but disabled by default in upstream. Is it still necessary to have it disabled by default in upstream? Can we get rid of conditional build?

Related tickets:
https://pagure.io/SSSD/sssd/issue/2439
https://bugzilla.redhat.com/show_bug.cgi?id=988068#c13

Uhm so why is this NSS_STATUS_SUCCESS ?

This doesn't look right:
5 - if (!buffer || !buflen) return ERANGE;
6 + if (!buffer || !buflen) {
7 + » *errnop = ERANGE;
8 + » return NSS_STATUS_SUCCESS;
9 + }

Correct, this was a mistake on my part, just a mental typo. Thanks for catching this.

Could you also check other parts in the same file.
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_734
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_749
https://pagure.io/SSSD/sssd/blob/master/f/src/sss_client/common.c#_769
NSS_STATUS_NOTFOUND is returned only with enabled switch --enable-sss-default-nss-plugin. It is enabled in fedora and el7 but disabled by default in upstream. Is it still necessary to have it disabled by default in upstream? Can we get rid of conditional build?

Related tickets:
https://pagure.io/SSSD/sssd/issue/2439
https://bugzilla.redhat.com/show_bug.cgi?id=988068#c13

This is a distinct issue and should be discussed in another forum. Please feel free to email me aobut this issue or file a new PR and CC me.

1 new commit added

  • nss: Fix invalid enum nss_status return values.

Updated my fork with the fixed NSS status code. Should be good now.

Tested local build OK for password service.

Tested x86_64 glibc + sssd only.

Tested x86_64 glibc + sssd only.

@codonell I ran some downstream tests and they passed as well.

Would you mind if I squash patches to a single patch before merging?

And I assume that this patch is not related to any version of glibc and therefore can be pushed also to older branches. Is it correct assumption?

Tested x86_64 glibc + sssd only.

@codonell I ran some downstream tests and they passed as well.
Would you mind if I squash patches to a single patch before merging?
And I assume that this patch is not related to any version of glibc and therefore can be pushed also to older branches. Is it correct assumption?

Yes, please squash the patches into a single commit.

The patch is OK to apply everywhere. It has no dependency on glibc versions.

The fix will help our glibc testing configurations that have sssd.

master:

  • c096972ff25f14a390a621851f4935901b908be8

sssd-1-14:

  • 7d917d8202efab9a3af273b192d842056593ac77

sssd-1-13:

  • d951b5b67042ab8e561ec501b27a7084cae025b1

Pull-Request has been closed by lslebodn

I'll do new update of sssd in fedora soon so I will backport this patch as well.

Thank you very much guys.

I'll do new update of sssd in fedora soon so I will backport this patch as well.
Thank you very much guys.

Thank you for helping us get the fix in place!

@codonell fedora will soon have fixed version in updates-testing. packages were pushed yesterday for f25+

Metadata