| Message ID | 1455705580-27781-1-git-send-email-ian.campbell@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2016-02-17 at 10:39 +0000, Ian Campbell wrote: > We assert that nullfd if not std{in,out,err} since that would result > in closing one of the just dup2'd fds. For this to happen > std{in,out,err} would have needed to be closed, at which point all > sorts of other things could go wrong. > > CID: 1130519 > > It was previously hypothesised[0] that fixing 1130516 would solve this > too, but that appears to not have been the case. > > Compile tested only. > > [0] http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg02931. > html > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > Cc: andrew.cooper3@citrix.com ping? > --- > v2: Assert logfile and nullfd are not stdio fds > --- > tools/libxl/xl_cmdimpl.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > index 9958d8a..a377de1 100644 > --- a/tools/libxl/xl_cmdimpl.c > +++ b/tools/libxl/xl_cmdimpl.c > @@ -499,12 +499,17 @@ static int do_daemonize(char *name, const char > *pidfile) > > CHK_SYSCALL(logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND, > 0644)); > free(fullname); > + assert(logfile >= 3); > > CHK_SYSCALL(nullfd = open("/dev/null", O_RDONLY)); > + assert(nullfd >= 3); > + > dup2(nullfd, 0); > dup2(logfile, 1); > dup2(logfile, 2); > > + close(nullfd); > + > CHK_SYSCALL(daemon(0, 1)); > > if (pidfile) {
On Tue, Feb 23, 2016 at 10:30:31AM +0000, Ian Campbell wrote: > On Wed, 2016-02-17 at 10:39 +0000, Ian Campbell wrote: > > We assert that nullfd if not std{in,out,err} since that would result > > in closing one of the just dup2'd fds. For this to happen > > std{in,out,err} would have needed to be closed, at which point all > > sorts of other things could go wrong. > > > > CID: 1130519 > > > > It was previously hypothesised[0] that fixing 1130516 would solve this > > too, but that appears to not have been the case. > > > > Compile tested only. > > > > [0] http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg02931. > > html > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > Cc: andrew.cooper3@citrix.com > > ping? Ian, you wouldn't have a git branch with all your outstanding patches you had posted somewhere? Just in case we don't get to them done by feature freeze window and somebody starts replaying these patches.. > > > --- > > v2: Assert logfile and nullfd are not stdio fds > > --- > > tools/libxl/xl_cmdimpl.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c > > index 9958d8a..a377de1 100644 > > --- a/tools/libxl/xl_cmdimpl.c > > +++ b/tools/libxl/xl_cmdimpl.c > > @@ -499,12 +499,17 @@ static int do_daemonize(char *name, const char > > *pidfile) > > > > CHK_SYSCALL(logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND, > > 0644)); > > free(fullname); > > + assert(logfile >= 3); > > > > CHK_SYSCALL(nullfd = open("/dev/null", O_RDONLY)); > > + assert(nullfd >= 3); > > + > > dup2(nullfd, 0); > > dup2(logfile, 1); > > dup2(logfile, 2); > > > > + close(nullfd); > > + > > CHK_SYSCALL(daemon(0, 1)); > > > > if (pidfile) { > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Mon, Feb 29, 2016 at 10:45:51AM -0500, Konrad Rzeszutek Wilk wrote: > On Tue, Feb 23, 2016 at 10:30:31AM +0000, Ian Campbell wrote: > > On Wed, 2016-02-17 at 10:39 +0000, Ian Campbell wrote: > > > We assert that nullfd if not std{in,out,err} since that would result > > > in closing one of the just dup2'd fds. For this to happen > > > std{in,out,err} would have needed to be closed, at which point all > > > sorts of other things could go wrong. > > > > > > CID: 1130519 > > > > > > It was previously hypothesised[0] that fixing 1130516 would solve this > > > too, but that appears to not have been the case. > > > > > > Compile tested only. > > > > > > [0] http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg02931. > > > html > > > > > > Signed-off-by: Ian Campbell <ian.campbell@citrix.com> > > > Cc: andrew.cooper3@citrix.com > > > > ping? > > Ian, you wouldn't have a git branch with all your outstanding > patches you had posted somewhere? > > Just in case we don't get to them done by feature freeze window and > somebody starts replaying these patches.. > Bug fixes are allowed to go in even after the freeze. Wei.
Ian Campbell writes ("[PATCH v2] xl: close nullfd after dup2'ing it to stdin"):
> We assert that nullfd if not std{in,out,err} since that would result
> in closing one of the just dup2'd fds. For this to happen
> std{in,out,err} would have needed to be closed, at which point all
> sorts of other things could go wrong.
>
> CID: 1130519
>
> It was previously hypothesised[0] that fixing 1130516 would solve this
> too, but that appears to not have been the case.
>
> Compile tested only.
>
> [0] http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg02931.html
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: andrew.cooper3@citrix.com
> ---
> v2: Assert logfile and nullfd are not stdio fds
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
(The copy to ijc may bounce I guess...)
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c index 9958d8a..a377de1 100644 --- a/tools/libxl/xl_cmdimpl.c +++ b/tools/libxl/xl_cmdimpl.c @@ -499,12 +499,17 @@ static int do_daemonize(char *name, const char *pidfile) CHK_SYSCALL(logfile = open(fullname, O_WRONLY|O_CREAT|O_APPEND, 0644)); free(fullname); + assert(logfile >= 3); CHK_SYSCALL(nullfd = open("/dev/null", O_RDONLY)); + assert(nullfd >= 3); + dup2(nullfd, 0); dup2(logfile, 1); dup2(logfile, 2); + close(nullfd); + CHK_SYSCALL(daemon(0, 1)); if (pidfile) {
We assert that nullfd if not std{in,out,err} since that would result in closing one of the just dup2'd fds. For this to happen std{in,out,err} would have needed to be closed, at which point all sorts of other things could go wrong. CID: 1130519 It was previously hypothesised[0] that fixing 1130516 would solve this too, but that appears to not have been the case. Compile tested only. [0] http://lists.xenproject.org/archives/html/xen-devel/2013-11/msg02931.html Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Cc: andrew.cooper3@citrix.com --- v2: Assert logfile and nullfd are not stdio fds --- tools/libxl/xl_cmdimpl.c | 5 +++++ 1 file changed, 5 insertions(+)