Summary: | getpwent() can set errno to ENOENT which is not documented | ||
---|---|---|---|
Product: | glibc | Reporter: | Laurent Bigonville <bigon> |
Component: | nss | Assignee: | Not yet assigned to anyone <unassigned> |
Status: | RESOLVED INVALID | ||
Severity: | normal | CC: | carlos, filbranden, nicolas.iooss_sourceware |
Priority: | P2 | Flags: | fweimer:
security-
|
Version: | 2.27 | ||
Target Milestone: | --- | ||
See Also: | https://github.com/systemd/systemd/issues/9585 | ||
Host: | Target: | ||
Build: | Last reconfirmed: | ||
Attachments: | Test case |
Description
Laurent Bigonville
2018-07-13 16:50:31 UTC
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. |