From: Stefan Sperling <stsp@stsp.name>
Subject: Re: [PATCH] privsep.c: ignore closefrom() return value
To: Anna “CyberTailor” <cyber+misc@sysrq.in>, gameoftrees@openbsd.org
Date: Mon, 6 Dec 2021 13:49:04 +0100

On Mon, Dec 06, 2021 at 11:54:13AM +0100, Stefan Sperling wrote:
> On Mon, Dec 06, 2021 at 03:19:04PM +0500, Anna “CyberTailor” wrote:
> > closefrom(2) is void on FreeBSD and libbsd, so its return value has to
> > be ignored.
> > 
> > https://bugs.gentoo.org/828003
> > ---
> >  lib/privsep.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> This patch should be applied to -portable only.
> 
> On OpenBSD there are failure conditions:
> 
>      closefrom() will fail if:
> 
>      [EBADF]		fd is greater than all open file descriptors.
> 
>      [EINTR]		An interrupt was received.
> 
> Especially EINTR seems important. It means closefrom was interrupted
> and could not complete its task.

Taking a closer look at this, the FreeBSD port uses a #define to
work around this:
https://cgit.freebsd.org/ports/plain/devel/got/files/openbsd-compat/openbsd-compat.h

/* void -> int */
#define closefrom(fd)			(closefrom(fd), 0)

Would this approach work for -portable?

> > diff --git a/lib/privsep.c b/lib/privsep.c
> > index ca518be1..8f233724 100644
> > --- a/lib/privsep.c
> > +++ b/lib/privsep.c
> > @@ -2766,10 +2766,8 @@ got_privsep_exec_child(int imsg_fds[2], const char *path, const char *repo_path)
> >  		fprintf(stderr, "%s: %s\n", getprogname(), strerror(errno));
> >  		_exit(1);
> >  	}
> > -	if (closefrom(GOT_IMSG_FD_CHILD + 1) == -1) {
> > -		fprintf(stderr, "%s: %s\n", getprogname(), strerror(errno));
> > -		_exit(1);
> > -	}
> > +
> > +	closefrom(GOT_IMSG_FD_CHILD + 1);
> >  
> >  	if (execl(path, path, repo_path, (char *)NULL) == -1) {
> >  		fprintf(stderr, "%s: %s: %s\n", getprogname(), path,
> > -- 
> > 2.34.1
> > 
> > 
> 
>