Created attachment 11127 [details] Test case Hi, A recent discussion on the selinux mailing list showed that in some conditions getpwent() can set errno to ENOENT. But this is not documented anywhere. The manpage for getpwent(3), shows the following possible errors: EINTR A signal was caught; see signal(7). EIO I/O error. EMFILE The per-process limit on the number of open file descriptors has been reached. ENFILE The system-wide limit on the total number of open files has been reached. ENOMEM Insufficient memory to allocate passwd structure. ERANGE Insufficient buffer space supplied. I attached a simple test case I can reproduce this with (you need to have the nss-systemd module loaded for the passwd database) Apparently a similar bug report was opened in the past but was closed as INVALID, see bug #1969 [0] https://marc.info/?l=selinux&m=153149269702678&w=2
Hi, I have dug into systemd's code to understand where this ENOENT comes from. It appears it is intentional: _nss_systemd_getpwent_r() sets errno to ENOENT if there is no item to return (https://github.com/systemd/systemd/blob/v239/src/nss-systemd/nss-systemd.c#L781): if (!p) { *errnop = ENOENT; ret = NSS_STATUS_NOTFOUND; goto finalize; } If returning with errno=ENOENT is acceptable, its semantic I agree its semantic should appear in the manpage of getpwent(3). Otherwise, this should be reported to systemd's developers. Moreover "getent passwd" does not report any failure from getpwent (in https://sourceware.org/git/?p=glibc.git;a=blob;f=nss/getent.c;hb=glibc-2.27#l583), which makes this issue a little bit harder to debug than it should (it requires a C program which checks errno after getting NULL from getpwent()). Would a patch which displays the error from getpwent() in getent.c be accepted? Thanks
NSS_STATUS_NOTFOUND is not an error condition and the nss module must not modify errno. Please report that to systemd.
Thanks for your feedback. I reported this issue to systemd: https://github.com/systemd/systemd/issues/9585
Well reading: https://www.gnu.org/software/libc/manual/html_node/NSS-Modules-Interface.html Returning NSS_STATUS_NOTFOUND and setting errno to ENOENT seems valid for a NSS module OTOH, getpwent(3) setting ENOENT is NOT documented So either the documentation needs to be fixed and/or clarified or getpwent() should reset errno
(In reply to Laurent Bigonville from comment #4) > Well reading: > > https://www.gnu.org/software/libc/manual/html_node/NSS-Modules-Interface.html > > Returning NSS_STATUS_NOTFOUND and setting errno to ENOENT seems valid for a > NSS module It's valid in the general sense, and the above documentation is a high-level overview of the NSS interface. However, the above is not a specific discussion of each interface being implemented. And the documentation calls that out: ~~~ These are proposed values. There can be other error codes and the described error codes can have different meaning. ~~~ > OTOH, getpwent(3) setting ENOENT is NOT documented Correct, because semantically it doesn't make sense. You either provide an entry or if there are no more entries you return NULL. The deeper question is: Is it valid for getpwent() to fail even if the implementation of NSS service module fails? I'd argue it doesn't make sense for the NSS service module to expose this error because the caller can't recover from it except to abort. > So either the documentation needs to be fixed and/or clarified or getpwent() > should reset errno Is the NSS_STATUS_NOTFOUND + ENOENT a *real* internal error condition for systemd's NSS service module? Is systemd trying to indicate that retrieving the next entry resulted in an error that it could not recover from? In the case of implementing getpwent it might be better for it to return NULL and no error.
Proposed patch to systemd here: https://github.com/systemd/systemd/pull/9587/files Your feedback would be welcome there, in particular there's a question of whether to set *errnop = 0; explicitly or just leave it to whatever the caller set it to... (Does that matter?) Updating those docs would be appreciated, as they currently stand, it's unclear whether (NSS_STATUS_NOTFOUND + ENOENT) is even valid at all and, if it is, in which situations should it be used (over NSS_STATUS_NOTFOUND + 0 errno or keeping errno untouched.) Cheers, Filipe
(In reply to Filipe Brandenburger from comment #6) > Proposed patch to systemd here: > > https://github.com/systemd/systemd/pull/9587/files > > Your feedback would be welcome there, in particular there's a question of > whether to set *errnop = 0; explicitly or just leave it to whatever the > caller set it to... (Does that matter?) It matters. Do not set it to zero. Please leave it if no real error is being reported. > Updating those docs would be appreciated, as they currently stand, it's > unclear whether (NSS_STATUS_NOTFOUND + ENOENT) is even valid at all and, if > it is, in which situations should it be used (over NSS_STATUS_NOTFOUND + 0 > errno or keeping errno untouched.) Thanks, we will keep this under consideration when updating the manual.
No library function should ever set errno to zero.