On Tue, Jun 25, 2019 at 04:55:10PM -0700, Michael Forney wrote:
> On 2019-06-25, Richard Ipsum <richardipsum_AT_fastmail.co.uk> wrote:
> > On Tue, Jun 25, 2019 at 03:44:57PM -0700, Michael Forney wrote:
> >> On 2019-06-25, Richard Ipsum <richardipsum_AT_fastmail.co.uk> wrote:
> >> > This fixes an error on OpenBSD,
> >> >
> >> > % chown 1000:1000 yes.c
> >> > ./chown: getgrnam 1000: No such file or directory
> >>
> >> POSIX says that if the entry cannot be found, errno shall not be
> >> changed. Are you saying that OpenBSD doesn't behave this way?
> >
> > Yeah when I run this on OpenBSD with uid:gid values I get ENOENT.
>
> Do you think that's something they'd be willing to fix?
>
> I think the error is coming from
> https://github.com/openbsd/src/blob/f84583fe5d7899ab1504dc1d58a04742db4a3058/lib/libc/gen/getgrent.c#L217.
> If I run `touch /var/run/ypbind.lock` in my OpenBSD VM, it fixes the
> issue.
I see, so to preserve the existing behaviour you'd want to save errno
before calling access, then restore afterward.
>
> I think this is unintended, since there appears to be some logic to
> save and restore errno in grscan(). Looks like it was introduced in
> https://github.com/openbsd/src/commit/b971f1acd7c34a49359ccefbe512e06f3826a939
> (first released in OpenBSD 5.9).
Right, exactly. I think the reason this doesn't break OpenBSD chown is
because it uses `gid_from_group` and the implementation[1] for that
actually ignores errno, if it didn't they might have spotted this issue
themselves.
Do you want to raise this with upstream or shall I?
>
> >>
[snip]
> >>
> >> It looks like there is no error with getpwnam, only with getgrname,
> >> right?
> >>
> >
> > Yes, but I would argue that realistically the only time this function errors
> > is
> > when the entry doesn't exist, we don't want to exit in that case,
> > instead try parsing it as a numeric uid and if that fails then exit.
>
> The POSIX getgrnam specification also lists a couple other possible
> errors: I/O error and out of file descriptors.
>
> Also, the chown specification specifically says
>
> If a numeric group operand exists in the group database as a group
> name, the group ID number associated with that group name shall be
> used as the group ID.
>
> If we aren't able to read /etc/group for some reason and the the group
> name for a number exists, we would end up using the wrong group ID.
>
Yes I can see why you would rather not ignore errno to be fair.
Maybe the better option is to see if the fix can be made in OpenBSD
and then I could rework these patches?
Thanks,
Richard
[1]:
https://github.com/openbsd/src/blob/master/lib/libc/gen/pwcache.c#L384
Received on Wed Jun 26 2019 - 12:31:10 CEST