Date: Fri, 10 Jun 2016 21:17:45 +0200 From: Mariusz Zaborski <oshogbo@FreeBSD.org> To: Jilles Tjoelker <jilles@stack.nl> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r301572 - in head/lib/libcasper: libcasper services/cap_dns services/cap_grp services/cap_pwd services/cap_random services/cap_sysctl Message-ID: <20160610191745.GA18133@jarvis> In-Reply-To: <20160610144728.GA85730@stack.nl> References: <201606080203.u5823rbD075793@repo.freebsd.org> <20160610144728.GA85730@stack.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
--CE+1k2dSO48ffgeK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 10, 2016 at 04:47:28PM +0200, Jilles Tjoelker wrote: > On Wed, Jun 08, 2016 at 02:03:53AM +0000, Mariusz Zaborski wrote: > > Author: oshogbo > > Date: Wed Jun 8 02:03:53 2016 > > New Revision: 301572 > > URL: https://svnweb.freebsd.org/changeset/base/301572 >=20 > > Log: > > Add flags to the Casper services. >=20 > > CASPER_SERVICE_STDIO - Casper will not close the first three descript= ors (stdin, > > stdout and stderr) this can be helpful for debugging. > > CASPER_SERVICE_FD - Capser will not close all other descriptors, this= can > > be useful for a filesystem service. >=20 > This reminds me that there are some common cases where it is wrong to > close descriptors you don't know about. The non-POSIX command > diff <(cmd1) <(cmd2) > that compares the outputs of the two commands, when executed with bash > that was compiled with the full /dev/fd visible (as in poudriere), will > actually run something like > diff /dev/fd/63 /dev/fd/62 > passing two file descriptors to pipes. >=20 > When created by a shell, these pathnames will start with /dev/fd/, but > people could create symlinks to these special files. This is why the CASPER_SERVICE_FD was added. :) > > [snip] > > +static void > > +stdnull(void) > > +{ > > + int fd; > > + > > + fd =3D open(_PATH_DEVNULL, O_RDWR); > > + if (fd =3D=3D -1) > > + errx(1, "Unable to open %s", _PATH_DEVNULL); > > + > > + if (setsid() =3D=3D -1) > > + errx(1, "Unable to detach from session"); >=20 > There is an implicit assumption here that stdnull() is only called from > a process that was forked off from here, since setsid() will not and > cannot work when called from a process that is already a session leader. >=20 > If the application is running from a shell, this setsid() will exclude > the process from most signals, including terminal ^C/^\/^Z, kill % and > hangups. More generally, this might make it more likely for the process > to hang around indefinitely after the parent is gone. I'm not sure but if the process descriptor not solve that? If we close all process descriptor to the process it should die then, so you need to kill just the process which is using service. > > + > > + if (dup2(fd, STDIN_FILENO) =3D=3D -1) > > + errx(1, "Unable to cover stdin"); > > + if (dup2(fd, STDOUT_FILENO) =3D=3D -1) > > + errx(1, "Unable to cover stdout"); > > + if (dup2(fd, STDERR_FILENO) =3D=3D -1) > > + errx(1, "Unable to cover stderr"); > > + > > + close(fd); >=20 > This was not broken by this commit, but fd should not be closed if it is > equal to STDIN_FILENO, STDOUT_FILENO or STDERR_FILENO. Yes you are in 100% right. Thanks, I will fix that. Thanks, --=20 Mariusz Zaborski oshogbo//vx | http://oshogbo.vexillium.org FreeBSD commiter | https://freebsd.org Software developer | http://wheelsystems.com If it's not broken, let's fix it till it is!!1 --CE+1k2dSO48ffgeK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXWxJVAAoJEN/ChBrixsVkC70QALCuuSYjjvOoNrNrAUszSvFV Fs565nImHWLQItVN1nndrMTTxaHG6RsYW+i/Md1xan0Pcn2o31MqqjhjLDBPy2+6 VwWVdN4YfBIox+rGRQ7S2e626ygkjRnYJVQwshHSjsZWVpCxp99YJC0wskF5Ykeu 7ioGatI2JSSLRkzKsQ+nAHeKGNgL4/hvLKCcGG8CcoAyIfUEOJ7SrB0WrtrJsxDo viPh8lQuDUI/kMiD4x7VjJ91keAtsFutmLz4SEZPQZe9K6IwmNi5i1WDfrMPan/Y XqeA15zrylKc4rzGQTfcL0TA/EtmvSwEWWE/cxOpgyyeg1shIXNEjNG90qQ6XdEB 7C1YvoSK/+YRpzyZSYzBwxq57SyOFtorFDpy5Otyg0C593fBexqvzyjeYIZhoIqR BXS6s7lfPgNxpN691dK9c9/XCgdvCMNr7HDAT/+1a2RAdO3KmBaWfAqPG1QA5a1C cc+q/dEwkS5Er7xJasDmMlO7M61QyrQFcLpOYFXxwimoiYv6ZLkP8uu2nRqUYlWz HZ5JCfSQc9OWmxfewghR2Acw+ZtntbgseFAa76qra9TzLv5HCQOaO6qUvn7WNedB 0JtBcLbyI+Ad+02muKkoo5oMHR8s6KR0K0FFnkzPjfrrxDRfmT/C4fAve4hXZHte 304DL9cyFUSmscTJQnrR =jY8w -----END PGP SIGNATURE----- --CE+1k2dSO48ffgeK--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160610191745.GA18133>